Skip to content
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

Merged
merged 10 commits into from
Oct 24, 2017
Merged

Conversation

fingolfin
Copy link
Member

This addresses part of issue #358 and fixes UpEnv when at the bottom of the backtrace. I plan to improve Where 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).

@fingolfin fingolfin added the kind: bug Issues describing general bugs, and PRs fixing them label Oct 18, 2017
@fingolfin fingolfin added this to the GAP 4.9.0 milestone Oct 18, 2017
@codecov
Copy link

codecov bot commented Oct 19, 2017

Codecov Report

Merging #1780 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            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
Impacted Files Coverage Δ
src/vars.c 71.79% <100%> (+0.02%) ⬆️
src/gap.c 59.07% <100%> (+2.23%) ⬆️
src/read.c 83.83% <100%> (+0.95%) ⬆️
src/vars.h 97.05% <100%> (ø) ⬆️
lib/record.g 34.25% <0%> (-37.97%) ⬇️
src/hpc/thread.c 46.64% <0%> (-0.4%) ⬇️
src/hpc/threadapi.c 34.17% <0%> (-0.29%) ⬇️
src/hpc/traverse.c 78.59% <0%> (-0.09%) ⬇️
lib/record.gi 67.24% <0%> (ø)
src/stats.c 75.61% <0%> (+0.13%) ⬆️
... and 7 more

@ChrisJefferson
Copy link
Contributor

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.

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.
@fingolfin
Copy link
Member Author

Tests are of course always nice to have, and I tried to add some, based on your test-error code, but run into some snags; sorry, don't quite recall what they were... well one was that I needed to tests input into the brk> prompt, which test-error did not allow, but I fixed that by switching it from using Read to read the test input data, to piping the input data into GAP's stdin. That allowed me to test DownEnv etc, but since one only see the output, not the input, the result was not that satisfactory.

Anyway, I'll see if I can resurrect this attempt and see if I can make it work

@fingolfin fingolfin merged commit 1195411 into gap-system:master Oct 24, 2017
@fingolfin fingolfin deleted the mh/misc3 branch October 24, 2017 21:55
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UpEnv and Where are broken
3 participants