Error handling: split stack frames over two lines; adjust more manual examples by fingolfin · Pull Request #6263 · gap-system/gap

fingolfin · 2026-03-11T13:27:18Z

Render visible stack frames with numbered prefixes on one line and an aligned location line below, and give compiled GAP frames a stacktrace-specific format.

Also update a bunch of examples in the manual and code comments to reflect the new stack trace formatting.

AI-assisted with Codex for implementation, tests, and documentation.

Follow up to PR #6257

There are several aspects to this PR one may wish to discuss

It splits each stack trace line into two; with the file/location on its own line. I borrowed the formatting from Julia, because I know and like it, but am completely open to alternatives. I would also understand if people prefer the more compact old printing; but for me, it is often difficult to weed through a long and complex GAP stack trace, and often what I want most are the file:line pairs (as I just cmd-click on them in my terminal to look at the code they reference).
It updates more examples, and in some cases edits them; in some cases I decided it is best to just delete something. Please have a look if you agree
The implementation: I feel PRINT_CURRENT_STATEMENT gets more and more complex. I've thought about converting parts of it to GAP code, but this always ends up raising questions about "but won't this need to allocate or cause additional errors, and we can afford neither". So I decided to leave it as it is, to be refactor in a future PR. But feel free to disagree / suggest better alternatives.

Further ideas (for follow-up PRs):

investigate using color or bold style (if available; possibly configurable in the same way the color prompts are)
show the name of the active function in addition or even instead of the statement
- pro: the function name provides extra context
- pro: this is like every (?) other system seems to do it (e.g. gdb, Julia, Python)
- con: adding a third line (or filling up one of the existing lines) is not only a blessing
- hence why I mention that this could also be instead of showing the active statement/expression... but again that's not a clear win/loss:
  - the statement can be multiple lines or just generally very long, which is confusing
  - it also can be formatted differently from how it appears in the code
  - still it can be helpful in pinning down to which place exactly the stack trace refers... especially if the code was edited after being loaded, so that line numbers are off, then this is nice for "adjusting" to that
change DownEnv/UpEnv to always re-print the stacktrace
- if we do this, then IMHO it should be possible to disable this via a user config
- could be similar to WhereDepths (see Add WhereDepth user preference to control depth of initial stack trace in break loops #6261), something like UpDownEnvWhereDepths: if set to 0 or negative, then no Where is executed, any positive value runs Where with that many levels; if the setting is not set at all, default to WhereDepth ?)
... and maybe more, your ideas here

fingolfin · 2026-03-11T13:28:15Z

doc/ref/debug.xml

 gap> dive(100);
-gap> OnBreak:= function() Where(1); end; # shorter traceback
-function(  ) ... end
+gap> OnBreak:= function() Where(2); end;; # shorter traceback


the "1" showed 2 level due to a bug that I fixed in #6257, so I've now changed it to 2 levels to more closely match was the example showed before

doc/ref/debug.xml

doc/ref/grpperm.xml

doc/ref/language.xml

doc/ref/mloop.xml

lib/grpfp.gd

lib/oper1.g

lib/smgideal.gi

fingolfin · 2026-03-11T13:51:10Z

src/error.c

        if (activeContext != Fail) {
-            Pr(context == activeContext ? "*[%d] " : " [%d] ",
-               INT_INTOBJ(level), 0);
+            snprintf(prefix, sizeof(prefix), "%c%*s[%lu] ",


We already use fprintf in this file, so I thought it's OK to allow the AI this ...

fingolfin · 2026-03-11T13:51:37Z

src/error.c

+                Pr("<<compiled GAP function \"%g\">>", (Int)funcname, 0);
+            }
+            else {
+                Pr("<<compiled GAP function>>", 0, 0);


We deliberately duplicate some PrintKernelFunction functionality to get a more consistent output

limakzi · 2026-03-11T16:24:26Z

src/error.c

+
+        while (totalDepthInt > 0) {
+            prefixWidth++;
+            totalDepthInt /= 10;


@fingolfin Do you like this prefix change?

Not sure which part you mean? Whether I like the way this code is written (not particular, but it will be replaced in a follow-up), or the feature it enables (indenting the [1], [2] "prefix" so that if they go double-digits, they keep right aligned) ? That I like (and made it add), it really helps reading the stack trace if it has 10 or more levels.

limakzi · 2026-03-11T16:26:54Z

tst/testspecial/syntax-tree-error.g.out

 gap> 
 gap> func( );
-Error,
+Error, 


@fingolfin How about removing this whitespace at the end?

This is genuinely output by Error, so removing it here would cause a test failure. I guess we could try to get rid of it here, but I am not sure it's worth the effort: anyone who writes Error() hopefully only does that in test code, not in something shipping to users...

doc/ref/debug.xml

lib/smgideal.gi

limakzi · 2026-03-11T16:34:18Z

src/calls.c

    Obj body = BODY_FUNC(func);
    Obj filename = body ? GET_FILENAME_BODY(body) : 0;
    if (filename) {
+        // A precise location means this is pure kernel code.


@fingolfin Do you think these comments are valuable?

Yes and no. I explained to the AI what these different cases mean (so that it could adjust or copy them as needed); and then asked it to turn my remarks into comments, but it didn't do a good job: the first two are indeed "obvious" from the code context; and the third one is misleading: this is not a "legacy" case, but a matter of some code, here MAKE_BITFIELDS, producing function objects with arguably incomplete metadata.

I'll revise this.

Render visible stack frames with numbered prefixes on one line and an aligned location line below, and give compiled GAP frames a stacktrace-specific format. AI-assisted with Codex for implementation, tests, and documentation. Co-authored-by: Codex <codex@openai.com>

james-d-mitchell

I think this looks really good (in terms of the way the stack traces look, I haven't really looked at the implementation).

james-d-mitchell · 2026-03-12T20:54:25Z

tst/testspecial/stack-depth-rec.g.out

- [4] String( record.(nam) ) at GAPROOT/lib/record.gi:LINE called from
- [5] String( record.(nam) ) at GAPROOT/lib/record.gi:LINE called from
+*[1] String( record.(nam) )
+   @ GAPROOT/lib/record.gi:LINE


Is this really supposed to be LINE?

Yes. The script tst/testspecial/run_gap.sh intentionally substitutes the strings GAPROOT and LINE into these files; otherwise we'd constantly get failures caused by e.g. someone inserting a line into lib/record.gi.

james-d-mitchell · 2026-03-13T09:14:18Z

lib/integer.gd

-##  <function>( <arguments> ) called from read-eval-loop
-##  Entering break read-eval-print loop ...
+##  Error, the residues must be equal modulo 2
+##  *[1] Error( "the residues must be equal modulo ", g.gcd );


I find it slightly vexing to see the error message in line 251 and then again in 252, but I understand that this is a necessity.

Yeah, agreed...

So, there is a way: in other system, we wouldn't print the statement with the error here, we would print the function containing the statement... but I'd rather leave that to a follow-up PR, as I believe it may be controversial...

I think the idea of follow-up pull request is great.
I fell like @james-d-mitchell, but I see... the controversy. 😼
I would close it.

james-d-mitchell · 2026-03-13T09:16:55Z

tst/testspecial/backtrace.g.out

-*[1] <<compiled GAP code>> from GAPROOT/lib/oper1.g:LINE in function INSTALL_METHOD called from
- [2] <<compiled GAP code>> from GAPROOT/lib/oper1.g:LINE in function InstallMethod called from
+*[1] <<compiled GAP function "INSTALL_METHOD">>
+   @ GAPROOT/lib/oper1.g:LINE


More instances of "LINE" which should probably be a number no?

fingolfin · 2026-03-13T17:47:19Z

@james-d-mitchell @limakzi thank you for your reviews. I've now tried to address everything. Please user the "Resolve conversation" buttons suitably if you are satisfied; or else please feel free to ask follow-ups

limakzi

LGTM.

doc/ref/debug.xml

doc/ref/mloop.xml

limakzi · 2026-03-14T21:38:05Z

lib/integer.gd

-##  <function>( <arguments> ) called from read-eval-loop
-##  Entering break read-eval-print loop ...
+##  Error, the residues must be equal modulo 2
+##  *[1] Error( "the residues must be equal modulo ", g.gcd );


I think the idea of follow-up pull request is great.
I fell like @james-d-mitchell, but I see... the controversy. 😼
I would close it.

lib/smgideal.gi

fingolfin · 2026-03-24T22:39:55Z

Thanks again for your feedback, @limakzi, much appreciated. Will auto-merge this now and make a follow-up PR tomorrow or so.

fingolfin · 2026-03-24T22:40:52Z

@james-d-mitchell @ChrisJefferson happy with this? feel free to "approve" if you are ;-)

ThomasBreuer

Nice.

… examples (gap-system#6263) Render visible stack frames with numbered prefixes on one line and an aligned location line below, and give compiled GAP frames a stacktrace-specific format. AI-assisted with Codex for implementation, tests, and documentation. Co-authored-by: Codex <codex@openai.com>

fingolfin requested review from ChrisJefferson, james-d-mitchell and limakzi March 11, 2026 13:27

fingolfin added release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: error handling labels Mar 11, 2026