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

make last2 and last3 available in a break loop #2375

Merged
merged 1 commit into from
Apr 19, 2018

Conversation

cdwensley
Copy link
Contributor

This PR addresses issue #2374.
I came upon this by accident - it is not a feature that I particularly wish to use.
Only 'last' is currently available inside a break loop because LastDepth=1 is set there.
The fix suggested by @ChrisJefferson is to change '1' to '3' in lib/error.g line 240.
Alternatively we could add "(Note that last2 and last3 are not available inside a break loop.)" to the reference manual.
The additional suggestion by @stevelinton, "Would it be nicer if the old values of last* were reset on return from the break loop?", has not been addressed.

@codecov
Copy link

codecov bot commented Apr 18, 2018

Codecov Report

Merging #2375 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2375      +/-   ##
==========================================
- Coverage   73.61%   73.61%   -0.01%     
==========================================
  Files         484      484              
  Lines      245759   245759              
==========================================
- Hits       180910   180908       -2     
- Misses      64849    64851       +2
Impacted Files Coverage Δ
lib/error.g 35.79% <0%> (ø) ⬆️
hpcgap/lib/hpc/stdtasks.g 64.19% <0%> (-0.77%) ⬇️
src/hpc/threadapi.c 43.4% <0%> (+0.1%) ⬆️

@ChrisJefferson
Copy link
Contributor

While there are many other changes we could make, this fixes an obvious bug (at the moment we change last, but not last2 and last3 in break loops, which is just silly)

@fingolfin
Copy link
Member

Seems OK to me, though of course as always, a test case would be nice. I.e. add a new file to tst/test-error/, say last-access.g. In it, repeat the inputs you used, like this:

1;2;3;[last,last2,last3];
Error("err");
1;2;3;[last,last2,last3];

Then, run the regenerate_error_tests.sh script in that dir. It should produce a file last-access.g.out which you also need to add to the repository. For me, it has this content (without your fix, so the final one will be different):

gap> 1;2;3;[last,last2,last3];
1
2
3
[ 3, 2, 1 ]
gap> Error("err");
Error, err called from
not in any function at *stdin*:3
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk> 1;2;3;[last,last2,last3];
1
2
3
[ 3, 3, 2 ]
brk> QUIT;

That said, it's also fine to merge this as-is, and we'll add a test later.

@cdwensley
Copy link
Contributor Author

Made a test-error/last-access.g as described above, but ./regenerate_error_tests.sh messed up all the *.out files (and created *.out-e files) by putting brk> in place of gap> and brk-2> in place of brk>, e.g. last-access.g.out looks like this:
brk> 1;2;3;[last,last2,last3];
1
2
3
[ 3, 2, 1 ]
brk> Error("err");
Error, err at GAPROOT/lib/package.gi:1836 called from
func( ); at GAPROOT/lib/system.g:208 called from
<function "CallAndInstallPostRestore">( )
called from read-eval loop at errin:3
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk_2> 4;5;6;[last,last2,last3];
4
5
6
[ 6, 5, 4 ]
brk_2> QUIT;
Not sure what to do about this!

@fingolfin
Copy link
Member

Don't worry about it.

@fingolfin fingolfin merged commit e027dd8 into gap-system:master Apr 19, 2018
@fingolfin fingolfin added the kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements label May 2, 2018
@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Sep 28, 2018
@cdwensley cdwensley deleted the last2last3 branch January 8, 2019 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements 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.

3 participants