Gauche icon indicating copy to clipboard operation
Gauche copied to clipboard

please keep track of "include" in error messages

Open pclouds opened this issue 4 years ago • 4 comments

def.sld contains

(define-library (def)
  (import (scheme base))
  (include "def.scm"))

and def.scm contains

(define reverse-rconj
  (case-lambda
    (() '())
    ((lst) lst)
    ((lst x) (cons x lst))))

And importing def, I got this error message

$ gosh -I.
gosh> (import (def))
*** ERROR: invalid application: (() ())
    While loading "./def.sld" at line 3
    While compiling: (import (def))
    While compiling "(standard input)" at line 1: (import (def))

A real world def.scm file is much bigger and harder to see what exactly is "invalid application" here.I would expect it shows some line in def.scm instead of stopping at def.sld. And with r7rs promoting spliting sld/scm, we may see these messages more and more often.

pclouds avatar Sep 09 '19 10:09 pclouds

Your point is reasonable. Note that include is fully expanded at read-time, before any compilation/evaluation takes place, so at the time of error there's no dynamic environment indicating we're processing included file.

However, we do attach debug info to S-expression, which includes the original file name (def.scm). So what's missing here is that "invalid application" not showing that info. I need to look into why.

shirok avatar Sep 09 '19 10:09 shirok

Hmm, loaddoes show def.scm in the stack trace. Something's off with import.

osh> ,l ./def.sld
*** ERROR: invalid application: (() ())
    While loading "./def.sld" at line 3
Stack Trace:
_______________________________________
  0  (() '())
        at "./def.scm":3
  1  (case-lambda (() '()) ((lst) lst) ((lst x) (cons x lst)))
        at "./def.scm":2
  2  (eval expr env)
        at "../lib/gauche/interactive.scm":269

shirok avatar Sep 09 '19 12:09 shirok

I found that Scm_Load without SCM_LOAD_PROPAGATE_ERROR flag drops stack trace.

I made a following patch for do_require function in src/load.c .

--- load_orig.c	2019-08-12 21:01:38.000000000 +0900
+++ load.c	2019-09-18 14:52:30.521208400 +0900
@@ -873,7 +873,8 @@
     ScmLoadPacket xresult;
     ScmModule *prev_mod = vm->module;
     vm->module = base_mod;
-    int r = Scm_Load(Scm_GetStringConst(SCM_STRING(feature)), 0, &xresult);
+    int r = Scm_Load(Scm_GetStringConst(SCM_STRING(feature)),
+                     SCM_LOAD_PROPAGATE_ERROR, &xresult);
     vm->module = prev_mod;
     if (packet) packet->exception = xresult.exception;
 

Then, result is as follows.

>gosh -I.
gosh> (import (def))
*** ERROR: invalid application: (() ())
    While loading "./def.sld" at line 3
    While compiling: (import (def))
    While compiling "(windows console standard input)" at line 1: (import (def))

Stack Trace:
_______________________________________
  0  (() '())
        at "./def.scm":3
  1  (case-lambda (() '()) ((lst) lst) ((lst x) (cons x lst)))
        at "./def.scm":2
  2  (eval expr env)
        at "C:\\Program Files (x86)\\Gauche\\share\\gauche-0.97\\0.9.9_pre1\\lib
/gauche/interactive.scm":269
gosh[gauche.require-base]>

But, this patch breaks something (Scm_Load never returns when error occurs?).

So, it's not a solution but just a information ...

Hamayama avatar Sep 18 '19 08:09 Hamayama

I now remember the context. I have a plan to attach the stack trace informationt to the condition object when an exception is raised. Then the stack trace information is in xresult.exception, so it will be carried over.

Extracting stack trace is an expensive operation, however, and we don't want to do it every time a condition is thrown, for it would be a waste if the raised exception is caught as an expected flow.

If we had full-heap stack (meaning allocating stack frame in heap), saving the stack trace information is just retaining the continuation pointer, so it would be nothing. However, we use our own stack for optimization, and since the continuation frames in the stack can be moved, we need to track every pointer that points into the continuation frame in the stack area. That part is trickly so I've been putting off. Maybe it's time to do it.

shirok avatar Sep 24 '19 07:09 shirok