-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UpEnv and Where are broken #358
Comments
I think UpEnv works, but Where doesn't, so it looks like it hasn't. The problem seems to be the kernel and GAP level variables ErrorLVars getting out of sync. On entry to a break loop they both get set to the CurrentLVars (stack frame) but there doesn't seem to be anything to keep them in sync. The "right" solution, I think, is to move DownEnv and UpEnv to the library and get rid of the kernel version. There is an API which probably exposes enough of the stack frames to do this. Some care is needed to make sure that variable lookup in the break loop works as expect though (for at least some expectations). I'll look into it. |
This is a really annoying bug -- I never reported it, because I thought it was just meant that way, but looking at the manual, it is clear that this is supposed to work better. The current setup makes debugging really frustrating. Any improvement here would be highly welcome by me. BTW, I'd actually prefer if the output of
could we perhaps have something like this?
Here, each level has a number; we always see all levels; and the "active" level is marked with an asterisk. The one drawback this might have is with very deep backtraces, where it might be better to cut off the backtrace. If that is really a concern, I'd still think showing the levels would be helpful. Also, perhaps we could have a heuristic: If there are fewer than, say, 10 levels, always show all, with a marker. If there are more, then abbreviate it with some heuristic, perhaps like this:
|
Agreed. I got someway through digging into it, but I need to look again. The code (which I wrote) is rather ugly. |
I have looked into this, and in the course made some cleanups and did some refactoring, see PR #934. As to UpEnv(): It definitely is broken, and this is not a matter of ErrorLVars in kernel and in the library getting out of sync. Rather, it is caused by Moreover, I noticed another problem: I need to stop working on this for now, as I have other things to attend to (like preparing my lecture tomorrow ...). But one last thing: I also have a mostly working patch which impements a nicer backtrace, as displayed above. The only caveat is that the information on level #0 (i.e. the place where the error originated) very quickly gets corrupted: Just enter an unassigned variable name; this triggers a syntax "error", which in turn causes the kernel to invoke |
(and I feel I don't know enough just now about how LVars are handled, so i won't try to attempt that last hypothetical solution; in particular, it might be totally bonkers to start with -- I wouldn't know... yet... :-) |
@fingolfin , thanks for looking into this. While you are there, I was trying to use I've also never understood the relationship between |
@ChrisJefferson What exactly is your problem with Anyway, if you suspect there is a problem, I think it'd be best to open a separate issue. And I also was and am confused by |
I tried doing the example of DownEnv/UpEnv from section 6.5 of the manual, and found 2 issues:
If you perform DownEnv, the output of Where doesn't change (the documentation suggests the backtrace should move to wherever we are now in the environment).
UpEnv(n) always moves all the way up to the top, regardless of what value of n is passed.
I hope to get around to looking at these, but if someone else does first, great. They seem to have been broken since at least 4.7.0.
The text was updated successfully, but these errors were encountered: