Hi Andy, Andy Wingo <wi...@pobox.com> writes:
> Some comments: > > -#define SCM_VTABLE_FLAG_RESERVED_0 (1L << 5) > -#define SCM_VTABLE_FLAG_RESERVED_1 (1L << 6) > +#define SCM_VTABLE_FLAG_SIMPLE (1L << 5) /* instances of this vtable > have only "pr" fields */ > +#define SCM_VTABLE_FLAG_SIMPLE_RW (1L << 6) /* instances of this vtable > have only "pw" fields */ > > The comments do not appear to be correct, given a later check: > > + if (SCM_LIKELY (SCM_VTABLE_FLAG_IS_SET (vtable, SCM_VTABLE_FLAG_SIMPLE) > + && SCM_VTABLE_FLAG_IS_SET (vtable, > SCM_VTABLE_FLAG_SIMPLE_RW) > + && p < SCM_STRUCT_DATA_REF (vtable, > scm_vtable_index_size))) > + /* The fast path: HANDLE is a struct with only "p" fields. */ > + data[p] = SCM_UNPACK (val); > > It seems that currently SIMPLE is for all pr *or* all pw slots. True. > But we should be more orthogonal than that; let's have SIMPLE be for > having all slots be readable p slots. (Note that this still allows a > mix of readable and writable slots.) > > Then perhaps we can change SIMPLE_RW to be MUTABLE or something, to > indicate that all slots of this object are mutable. > > What do you think? I agree and I’ll look into it. Thanks for the review! Ludo’.
