Re: MPS image cache

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Emacs Devel <emacs-devel@gnu.org>
> Date: Sun, 05 May 2024 09:02:17 +0200
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > I'm plannign to do the following if nobody stops me:
> >
> > Both face an image cache are hash tables containing 2 arrays of pointers
> > to MPS objects (face, image).
> >
> > I want to introduce a new MPS object type representing such an array of
> > pointers, IGC_OBJ_PTR_VEC. The igc_header gives us the size of the
> > array, and being an MPS object, we get exclusive access to its contents.
> >
> > WDYT
> 
> Pushed that, but not yet using it. We can always remove that later...

It breaks the 32-bit build:

  In file included from lisp.h:42,
                   from igc.c:32:
  igc.c: In function 'fix_ptr_vec':
  ../lib/verify.h:213:41: error: static assertion failed: "verify (sizeof *h == 
sizeof *v)"
    213 | # define _GL_VERIFY(R, DIAGNOSTIC, ...) _Static_assert (R, DIAGNOSTIC)
        |                                         ^~~~~~~~~~~~~~
  ../lib/verify.h:330:20: note: in expansion of macro '_GL_VERIFY'
    330 | # define verify(R) _GL_VERIFY (R, "verify (" #R ")", -)
        |                    ^~~~~~~~~~
  igc.c:107:30: note: in expansion of macro 'verify'
    107 | #define igc_static_assert(x) verify (x)
        |                              ^~~~~~
  igc.c:1085:5: note: in expansion of macro 'igc_static_assert'
   1085 |     igc_static_assert (sizeof *h == sizeof *v);
        |     ^~~~~~~~~~~~~~~~~
  Makefile:453: recipe for target `igc.o' failed

And I truly don't understand how it was supposed to work:

  static mps_res_t
  fix_ptr_vec (mps_ss_t ss, void *client)
  {
    MPS_SCAN_BEGIN (ss)
    {
      struct igc_header *h = client_to_base (client);
      void **v = client;
      igc_static_assert (sizeof *h == sizeof *v);  <<<<<<<<<<<<<<<<<

Here '*v' is a pointer, whereas '*h' is 'struct igc_header', which is
a structure whose size is larger than 32 bits:

  enum
  {
    IGC_TYPE_BITS = 5,
    IGC_PVEC_BITS = 6,
    IGC_HASH_BITS = 21,
    IGC_SIZE_BITS = 32
  };

  struct igc_header
  {
    enum igc_obj_type obj_type : IGC_TYPE_BITS;
    enum pvec_type pvec_type : IGC_PVEC_BITS;
    mps_word_t hash : IGC_HASH_BITS;
    mps_word_t nwords : IGC_SIZE_BITS;
  };

So this can only work on a 64-bit architecture, and even then only if
the compiler packs the structure (which is entirely not guaranteed
AFAIK).  Or what am I missing here?

And wasn't it supposed to say

      void **v = &client;

instead?

IMNSHO, this kind of code really needs comments to explain what it
does and why.