Re: /srv/bzr/emacs/trunk r103781: * src/bytecode.c (Fbyte_code): CAR and CDR can GC.

[Stefan Monnier]

> Wait, what?  How?  wrong_type_argument never returns.  If we run
> into a problem, we'll throw up to some higher context and never hit the
> AFTER_POTENTIAL_GC case.

That's true the AFTER_POTENTIAL_GC will never be executed.

> BEFORE_POTENTIAL_GC (if it's not a noop) sets the
> stack top field to point to the top of the stack, but because we never
> return from wrong_type_argument, we don't care whether the stack is lost.

But even if we never return to this code, the GC will still want to
trace through the byte_code_stacks and will need the `top' field
for that.

> What am I missing?

Without the BEFORE_POTENTIAL_GC you hit the following check in
mark_byte_stack:

      /* If STACK->top is null here, this means there's an opcode in
         Fbyte_code that wasn't expected to GC, but did.  To find out
         which opcode this is, record the value of `stack', and walk
         up the stack in a debugger, stopping in frames of Fbyte_code.
         The culprit is found in the frame of Fbyte_code where the
         address of its local variable `stack' is equal to the
         recorded value of `stack' here.  */
      eassert (stack->top);

You could argue that the check is conservative, but it's actually
a useful check and the BEFORE_POTENTIAL_GC I added is very cheap (`car'
and `cdr' very rarely signal errors and the cost of such signalling
dwarfs the cost of BEFORE_POTENTIAL_GC).  You could also argue that instead
of BEFORE_POTENTIAL_GC we should simply set .top to .bottom so that the
elements on the stack can be GC'd.  If so, I'd agree and would encourage
you to make a patch for it.
Oh, and BTW, currently BEFORE_POTENTIAL_GC and AFTER_POTENTIAL_GC are
usually no-ops because the byte-code stack is stack-allocated and the
stack is already conservatively scanned on most systems.  But we could
significantly speed up Elisp function calls if we manage the byte-code
stacks differently (bigger chunks rather than one per function
invacation), so BEFORE/AFTER_POTENTIAL_GC may lose their no-op status in
the future.


        Stefan