Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about C code not calling quit or GCing

From: Matt Armstrong <[email-masked]>
Date: Wed, 10 Mar 2021 10:28:52 -0800

Pip Cet <[email-masked]> writes:

Hey Pip, just minor comments from me.

(I assume you already plan to put the use of the cleanup attribute
behind conditional macros for portability)

Why a linked list/stack implementation?  How about a global count var
that is incremented, decremented, and asserted zero?

You wrote that this impl depends on the stack direction, but I can't
figure out why.  If it is indeed the case, add a comment explaining
this?

As a macro name, I think something like ASSERT_NO_GC_IN_SCOPE would be
clearer.

Signaling that this is a "magical" scope based construct is useful
because this sort of thing is so unusual in C.  The first thing I looked
for was an "END_SCOPE" macro and started scratching my head.

I'd also use a clear that indicates a debug time check (ASSERT,
CRASH_IF, etc.)

"Don't allow" states an invariant but does not clearly indicate a
consequence or other intent.  It could imply something as polite as "GC
is disabled for this scope".

For the C level stuff, maybe call it gc_forbidden_scope?

> + /* Do not wrap into do { } while (0). */

Move the comment next to the #define.  Ideally, don't issue a command
for the next programmer but instead explain why the code is the way is.