Subject: bug#66912: With `require', the byte compiler reports the wrong file for errors.

From: Stefan Monnier <[email-masked]>
Date: Sun, 03 Nov 2024 21:46:42 -0500

Hi Alan,

I couldn't properly review your code because I had trouble
understanding it.  So instead, it's mostly questions.

> +static Lisp_Object
> +internal_cc_hb (enum handlertype handlertype,
> +		Lisp_Object (*bfun) (void), Lisp_Object handlers,
> +		Lisp_Object (*hfun) (Lisp_Object))
> +{
> +  struct handler *c = push_handler (handlers, handlertype);
> +
> +  if (handlertype == HANDLER_BIND)
> +    {
> +      c->val = Qnil;
> +      c->bin_handler = hfun;
> +      c->bytecode_dest = 0;
> +    }
> +  if (sys_setjmp (c->jmp))
> +    {
> +      Lisp_Object val = handlerlist->val;
> +      clobbered_eassert (handlerlist == c);
> +      handlerlist = handlerlist->next;
> +      return hfun (val);
> +    }
> +  else
> +    {
> +      Lisp_Object val = bfun ();
> +      eassert (handlerlist == c);
> +      handlerlist = c->next;
> +      return val;
> +    }
> +}

I don't understand the above code: `handler-bind` is not supposed to
call setjmp/longjmp: when the handler of a `handler-bind` gets called,
the stack is not unwound.
So what is the `sys_setjmp` for when `handlertype == HANDLER_BIND`?

> @@ -1900,9 +2066,12 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool continuable)
>  	        max_ensure_room (20);
>  	        push_handler (make_fixnum (skip + h->bytecode_dest),
>  	                      SKIP_CONDITIONS);
> -	        call1 (h->val, error);
> -	        unbind_to (count, Qnil);
> -	        pop_handler ();
> +		if (NILP (h->val))
> +		  (h->bin_handler) (error);

[ We don't need parens around `h->bin_handler`.  ]

> +		else
> +		  call1 (h->val, error);
> +	        unbind_to (count, Qnil); /* Removes SKIP_CONDITIONS handler.  */
> +	        pop_handler ();		 /* Discards HANDLER_BIND handler.  */

These comments don't match my understanding of the code: IIRC the
`pop_handler` pops the `SKIP_CONDITIONS` handler.  Also there's no
reason to presume the HANDLER_BIND handler is at the top, so if we
wanted to remove it, we'd have to work harder.

> +/* Handle errors signalled by the above function.  */
> +static Lisp_Object
> +rel_error_handler (Lisp_Object err)
> +{
> +  if (!NILP (Vdebug_on_error))
> +    {
> +      call_debugger (err);
> +      Fthrow (Qtop_level, Qt);
> +    }
> +  else
> +    return err;
> +}

I don't understand that.
AFAICT, this is the only HANDLER_BIND handler you install from
C code, right?  So this is the thing that motivates all the changes like
`internal_cc_hb`, etc... so clearly it must do something worthwhile, but
I don't see what it is.
How is this related to `Fprefix_load_file_names`?

        Stefan