-
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
kernel: reject invalid AND-filters #2756
kernel: reject invalid AND-filters #2756
Conversation
7c47c48
to
2506004
Compare
2506004
to
1efc8ad
Compare
Codecov Report
@@ Coverage Diff @@
## master #2756 +/- ##
==========================================
- Coverage 83.77% 83.77% -0.01%
==========================================
Files 681 681
Lines 346620 346651 +31
==========================================
+ Hits 290368 290392 +24
- Misses 56252 56259 +7
|
I cross-referenced this in #2804 under "PRs blocked by packages". |
The QPA-team has now created a new release 1.28 incorporating the fix. |
QPS: @sunnyquiver thanks, I see gap-packages/qpa#16 merged. There was another problem with the new release of QPA, so waiting now for the next one. XModAlg: ticked the box, gap-packages/xmodalg#25 is now in version 1.17 from 30/08/2018. homalg: ticked the box for homalg-project/homalg_project#205 merged in MatricesForHomalg and released in version 2018.08.25 dated 25/08/2018. |
@fingolfin all three packages are now in bootstrapping package archives - please rebase, and I hope we will be able to merge this after the test (hope no other packages did this in the meantime). |
1efc8ad
to
11c215a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Checked package loading log https://travis-ci.org/gap-system/gap/jobs/440558613 - this PR does not trigger any errors when packages are loaded.
Will this consist of three commits, or two of them will be glued together?
I'll squash them, and also look into fixing the compiler variant. |
ec4fa8a
to
6d3ba85
Compare
Also unify treatment of 'and' for filters in interpreter and executor. Note that previously, this input was silently accepted: gap> Center and IsAssociative; <Filter "<<and-filter>>"> This was effectively treated equivalently to `HasCenter and IsAssociative`. We now reject such code.
6d3ba85
to
58d256f
Compare
This rejects invalid AND-filters like this:
This contains PR #2755 and thus further extends the work begun in PR PR #2732. However, it should not be merged until fixed versions of packages relying on the old buggy behavior are part of the distribution. Specifically, these packages (they all already merged a fix):
Alternatively, we could skip merging PR #2755, and directly merge a (squashed) version of this PR. That depends a bit on whether we decide to merge PR #2755 for GAP 4.10 or not, and/or on how quick fixed releases of these packages appear.