Fix passing invalid rnams to records #3491
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChrisJefferson
added
the
kind: bug: crash
ChrisJefferson
force-pushed
the
fix-rec
branch
2 times, most recently
from
ad8de30
to
e091884
Compare
wilfwilson
approved these changes
Jun 5, 2019
wilfwilson
added
kind: bug
fingolfin
approved these changes
Jun 5, 2019
| ## | ||
| gap> r; | ||
| rec( ) | ||
| gap> \.\:\=(r,RNamObj("y"), 2); |
There was a problem hiding this comment.
You could also use ASS_REC instead of \.\:\=, that might be a bit easier to grok. Same for ISB_REC, ELM_REC, UNB_REC, of course, though there it's perhaps less of a concern.
There was a problem hiding this comment.
I'd prefer to use .:= and friends, are those are what we actually document.
Codecov Report
@@ Coverage Diff @@
## master #3491 +/- ##
==========================================
- Coverage 85.43% 85.42% -0.02%
==========================================
Files 699 699
Lines 346762 346600 -162
==========================================
- Hits 296255 296069 -186
- Misses 50507 50531 +24
|
|
Cleaned up. In the cleaning up I find one more function (NameRNam) where I could use my new test, so I did. |
fingolfin
reviewed
Jun 11, 2019
tst/testinstall/kernel/records.tst
Outdated
| @@ -28,6 +28,8 @@ Error, NameRName: <rnam> must be a positive small integer (not the integer -1) | |||
| gap> NameRNam(fail); | |||
| Error, NameRName: <rnam> must be a positive small integer (not the value 'fail\ | |||
| ') | |||
| gap> NameRNam(2^29); | |||
| Error, NameRName: <rnam> must be a valid rnam (not the integer 536870912) | |||
There was a problem hiding this comment.
This error message (and several before it) still has the old, incorrect NameRName in it, which is why the Travis tests failed.
There was a problem hiding this comment.
sorry, been away from my computer. Now hopefully fixed and poked.
wucas
added
release notes: added
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3489
This could be backported to 4.10, but seeing how long it has been around for, I don't think that is required.
Text for release notes
Fix crashes when passing invalid arguments to functions for records: \., IsBound\., Unbind\. and \.\:\=