-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Fix JET-reported issues in BinaryPlatforms.triplet(::AbstractPlatform)
#59343
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 JET-reported issues in BinaryPlatforms.triplet(::AbstractPlatform)
#59343
Conversation
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.
I presume the issue is that these functions have very bad effects and so the compiler can't assume they're consistent?
julia> Base.infer_effects(Base.BinaryPlatforms.libgfortran_version, (Base.BinaryPlatforms.Platform,))
(!c,!e,!n,!t,!s,!m,!u,+o,!r)
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.
This issue isn't about effects. Since effect analysis isn't integrated into the current compiler's abstract interpretation routine, it can't propagate type constraints that can be deduced from accessing mutable fields.
…m)` (#59343) The issue is that JET reported that in `libgfortran_version(p).major` the first arg of `getproperty` could be `nothing`. This is already checked in the previous line, but in a way that the compiler cannot remember until that call. Putting it into a variable should fix that. It would be great if this could get backported to at least 1.12 (1.10 and 1.11 would also be great), since that is where people try to use JET for their packages, and this reduces the Base noise in the output.
Thanks for merging. Can we get a backport label here? |
I already backported this to 1.12-rc2 |
…m)` (#59343) The issue is that JET reported that in `libgfortran_version(p).major` the first arg of `getproperty` could be `nothing`. This is already checked in the previous line, but in a way that the compiler cannot remember until that call. Putting it into a variable should fix that. It would be great if this could get backported to at least 1.12 (1.10 and 1.11 would also be great), since that is where people try to use JET for their packages, and this reduces the Base noise in the output.
…m)` (#59343) The issue is that JET reported that in `libgfortran_version(p).major` the first arg of `getproperty` could be `nothing`. This is already checked in the previous line, but in a way that the compiler cannot remember until that call. Putting it into a variable should fix that. It would be great if this could get backported to at least 1.12 (1.10 and 1.11 would also be great), since that is where people try to use JET for their packages, and this reduces the Base noise in the output.
The issue is that JET reported that in
libgfortran_version(p).major
the first arg ofgetproperty
could benothing
. This is already checked in the previous line, but in a way that the compiler cannot remember until that call. Putting it into a variable should fix that.It would be great if this could get backported to at least 1.12 (1.10 and 1.11 would also be great), since that is where people try to use JET for their packages, and this reduces the Base noise in the output.