-
Notifications
You must be signed in to change notification settings - Fork 160
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
Fix UpEnv, some other lvars tweaks #1780
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1780 +/- ##
==========================================
+ Coverage 62.82% 62.82% +<.01%
==========================================
Files 969 970 +1
Lines 295193 295313 +120
Branches 13050 13051 +1
==========================================
+ Hits 185455 185544 +89
- Misses 106942 106959 +17
- Partials 2796 2810 +14
|
It would be nice to have some tests for this, but that could wait for later, we don't have any tests yet and testing it is going to require something like the 'test-error' tests. |
Now returns fail again
Also get rid of STATE(ErrorLVars0) This is an example for DownEnv/UpEnv usage: gap> f:=lvl -> 1/lvl + f(lvl-1); function( lvl ) ... end gap> f(3); Error, Rational operations: <divisor> must not be zero in return 1 / lvl + f( (lvl - 1) ); called from f( lvl - 1 ) called from f( lvl - 1 ) called from f( lvl - 1 ) called from <function "f">( <arguments> ) called from read-eval loop at line 12 of *stdin* you can replace <divisor> via 'return <divisor>;' brk> lvl; 0 brk> UpEnv(1); lvl; 0 brk> DownEnv(1); lvl; 1 brk> DownEnv(1); lvl; 2 brk> UpEnv(1); lvl; 1 brk> DownEnv(1); lvl; 2 brk> DownEnv(1); lvl; 3 brk> DownEnv(1); lvl; 3 brk> UpEnv(1); lvl; 2 Note that before this commit, the very last UpEnv(1) incorrectly returned us to level 0.
Tests are of course always nice to have, and I tried to add some, based on your Anyway, I'll see if I can resurrect this attempt and see if I can make it work |
This addresses part of issue #358 and fixes
UpEnv
when at the bottom of the backtrace. I plan to improveWhere
to indicate the current "level" inside the backtrace in a future PR, which then would fully resolve #358, I think.(Fixing
UpEnv
would be something for the release notes, I guess).