do not catch everything in isassigned #18075
Merged
+13
−3
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
|
Thanks, this was confusing when printing a broken custom array type gave lots of |
| catch | ||
| false | ||
| catch e | ||
| if isa(e, BoundsError) || isa(e, UndefRefError) |
There was a problem hiding this comment.
ok, i didn't realize that worked:
julia> isassigned([], 2)
false
lgtm
There was a problem hiding this comment.
But yeah, why not just use checkbounds for the bounds check part?
However, there doesn't seem to be anyway of checking if the field is undef without just trying to access it since that error is thrown from the runtime?
|
Because thats what the docs say. |
|
If you think this should be backported, do speak up. |
|
Not sure. It is a behaviour change so probably shouldn't? |
|
On the other hand it could be considered a bug fix since it now actually checks what the docs say it should check... |
StefanKarpinski
added
kind:bugfix
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.
If
getindexon anAbstractArrayerrors withThrowMeOrEveryoneWillPerishOhMyGodTheyAreComingExceptionwe should probably throw it instead of just returningfalseinisassigned.