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 some errors which didn't trigger a break loop anymore #3013

Merged

Conversation

fingolfin
Copy link
Member

Specifically use ErrorMayQuit instead of ErrorQuit in CheckIsPossList and
CheckIsDenseList to ensure this produces a break loop again:

f:=function()
local a,v;
  a:=[1.2,3];
  v:=2;
  v:=a{v};
end;
f();

Fixes #3011

I suggest to backport this to stable-4.10

Specifically use ErrorMayQuit instead of ErrorQuit in CheckIsPossList and
CheckIsDenseList to ensure this produces a break loop again:

    f:=function()
    local a,v;
      a:=[1.2,3];
      v:=2;
      v:=a{v};
    end;
    f();

Fixes gap-system#3011
@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them topic: kernel release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes backport-to-4.10 labels Nov 16, 2018
Copy link
Contributor

@hulpke hulpke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve of this being fixed but don't know enough about the kernel to say whether it is the right way.

Copy link
Member

@markuspf markuspf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly the fix I intended to fix #2920

@codecov
Copy link

codecov bot commented Nov 16, 2018

Codecov Report

Merging #3013 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3013      +/-   ##
==========================================
- Coverage   85.14%   85.14%   -0.01%     
==========================================
  Files         112      112              
  Lines       56959    56959              
==========================================
- Hits        48497    48495       -2     
- Misses       8462     8464       +2
Impacted Files Coverage Δ
src/error.c 79.61% <100%> (ø) ⬆️
src/saveload.c 66.04% <0%> (-0.54%) ⬇️

@ChrisJefferson ChrisJefferson merged commit 7d75e4c into gap-system:master Nov 17, 2018
@fingolfin fingolfin deleted the mh/more-error-break-loops branch November 17, 2018 23:53
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.10.1 milestone Nov 24, 2018
@olexandr-konovalov olexandr-konovalov added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.10-DONE 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 topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants