Re: [BDW-GC] Static allocation of subrs

[Neil Jerram]

address@hidden (Ludovic Courtès) writes:

>> Presumably that is in practice almost all of the primitives, though?
>
> That's 200 gsubrs, vs. 873 "simple" subrs.

Thanks, interesting.  That's more gsubrs than I expected.

>> Not so bad, IMO.
>
> It breaks cases like primitive procedures with more than 16 arguments
> (because I arbitrarily decided that `snarf-gsubr.h' would contain
> definitions up to 16 req/opt args), and situations like:
>
>   SCM_DEFINE (scm_foo, "foo", 0001, 0002, 0003, ...)
>
> This is probably acceptable in practice.

Agreed.  (In future maybe we might stop relying only on the C
preprocessor for snarfing.)

>> This part is quite dirty, as you say.  I'm not sure what's the
>> advantage of the generation at make time.  Wouldn't it be simpler, and
>> have the same function, just to hardcode all these definitions
>> directly in snarf.h?
>
> Given the size of `snarf-gsubr.h' (there are 16 * 16 * 2 = 512
> combinations), I'd rather keep it separated.  The makefile rule is an
> efficient way to compress it.  ;-)

OK.  In that case, can you add a comment in the Makefile.am to say
this?  Simply to avoid someone wondering in future if there is some
other reason why the code is auto-generated.

>> I see there's no NEWS in the commit; is that because there's no impact
>> on the API?  Even if so, I imagine it might merit a line in the 2.0
>> release notes.  If you agree, I'd encourage you to write the NEWS now,
>> rather than adding it later.
>
> Right.  It's just that I have not maintained any NEWS file for
> BDW-GC-related changes, initially.

I guess it's OK to add the NEWS later when this is merged; probably
your memory is better than mine!

>> Why does the macro code sometimes use scm_i_paste, and sometimes ##
>> directly?
>
> The `scm_i_paste ()' macro is needed so that macro-expansion of
> definitions like those of `srfi-4.i.c' work as expected.  There are a
> few cases where it isn't used (e.g., in `SCM_IMMUTABLE_DOUBLE_CELL ()'),
> but I guess it didn't cause any problem because `scm_i_paste ()' was
> properly used by callers.  We could unify that but `scm_i_paste ()' has
> the drawback of having a long name.

Ah, OK.  Thanks for explaining.

>> Finally SCM_SUBR_ARITY_TO_TYPE... The implementation feels a bit
>> messy, but I don't have any alternative to suggest, so I guess that's
>> OK.
>
> The problem is that this information is shared among 3 places:
> `SCM_SUBR_ARITY_TO_TYPE ()', `create_gsubr ()', and
> `SCM_DEFINE_SUBR_reqX_optY_rstZ ()'.  Ideally, all 3 would be generated
> from a single source.
>
> Besides, I think we need to review the use of subr/gsubr tag in detail
> to see whether we can make better use of them.

OK.  Nothing more needed straightaway, then.

>> But what about the fact that it's added to the libguile API?  I
>> realize that this is necessary to some extent so that snarf.h can be
>> used by application code - but can we somehow restrict
>> SCM_SUBR_ARITY_TO_TYPE to being used in that context, so that it
>> doesn't become on ongoing constraint for us?
>
> I share your concern, but I have no idea of how to avoid it.  We could
> add `_I_' in its name, perhaps.  Hopefully, it's too low-level for
> anyone to try it.

I don't have an idea either.  I think having _I_ would be good, though.

Regards,
      Neil