Skip to content

Conversation

@ChrisJefferson
Copy link
Contributor

Fixes RCWA.

This is undoing an earlier fix. The problem was that the ~ variable is not
cleared when exiting a break loop, or after an error. Reseting it's value
in ClearError fixed this, but it turns out ClearError is called even when there
are no errors, for example after reading a file.

This does reintroduce a tilde problem -- but it will only be hit by programs
which cause an error, then try to read tilde and find it has a value,
when it shouldn't.

A true fix will be reasonably irritating surgery, and still wouldn't properly fix the problem.
The real fix is to start storing, and restoring, tilde, every time we longjmp/setjmp, but the
cost of that might add up for such a little used feature (and it only matters after we have
an error, or exit the break loop).

@codecov
Copy link

codecov bot commented Feb 14, 2017

Codecov Report

Merging #1142 into master will decrease coverage by -0.28%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1142      +/-   ##
==========================================
- Coverage   57.39%   57.11%   -0.28%     
==========================================
  Files         434      434              
  Lines      230693   224797    -5896     
==========================================
- Hits       132403   128396    -4007     
+ Misses      98290    96401    -1889
Impacted Files Coverage Δ
src/stats.c 79.7% <ø> (+3.21%)
lib/grpramat.gi 56.9% <ø> (-22.66%)
lib/files.gi 29.09% <ø> (-17.63%)
src/macfloat.c 60.86% <ø> (-6.48%)
lib/files.gd 52.68% <ø> (-5.38%)
src/system.c 54.9% <ø> (-3.78%)
lib/csetperm.gi 87.65% <ø> (-3.71%)
src/objscoll.c 64.87% <ø> (-3.41%)
src/intfuncs.c 83.76% <ø> (-3.34%)
src/profile.c 34.76% <ø> (-2.74%)
... and 81 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8971a30...0e03a1a. Read the comment docs.

@olexandr-konovalov
Copy link
Member

@ChrisJefferson Please put a cross-ref to the earlier fix. Is it now "unfixed" fully or partially?

@ChrisJefferson
Copy link
Contributor Author

This removes part of #1050. Users still cannot assign ~, but when exiting a break loop, or some other situations, users may find it assigned at the top level, when it should not be.

This bug has been in GAP for at least 10 years, it's just no-one else noticed or cared before.

@fingolfin
Copy link
Member

@ChrisJefferson Could you please rebase this, so that the AppVeyor fixes are on this branch, and we can actually see the results of the AppVeyor tests? (Not that I think they'll fail, but it feels wrong to merge a "failin" branch ;-).

This is undoing an earlier fix. The problem was that the ~ variable is not
cleared when exiting a break loop, or after an error. Reseting it's value
in ClearError fixed this, but it turns out ClearError is called even when there
are no errors, for example after reading a file.

This does reintroduce a tilde problem -- but it will only be hit by programs
which cause an error, then try to read tilde and find it has a value,
when it shouldn't.
@fingolfin fingolfin merged commit 667e8bb into gap-system:master Feb 15, 2017
@ChrisJefferson ChrisJefferson deleted the tilde-fix branch May 12, 2017 11:26
@olexandr-konovalov olexandr-konovalov added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants