-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 ~370 invalidations from Expr(:ncat, ...) pretty-printing #41877
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.
LGTM. Should we backport this to 1.7 as well? Seems pretty harmless to me.
A codegen performance test seems to be unexpectedly unhappy with this, but LGTM |
How can I see the codegen performance test? I could get rid of the |
Yes, that might be preferable. A type assertion error seems more appropriate here than a conversion error in case someone generated an invalid AST. |
These get invalidated by loading Static.jl, specifically the method ``` Base.convert(::Type{T}, ::StaticInt{N}) where {T<:Number,N} = convert(T, N) ```
a218f6a
to
b66dbe1
Compare
It was probably just something transient and weird about CI at the time |
These get invalidated by loading Static.jl, specifically the method ``` Base.convert(::Type{T}, ::StaticInt{N}) where {T<:Number,N} = convert(T, N) ``` (cherry picked from commit 232ee11)
…ng#41877) These get invalidated by loading Static.jl, specifically the method ``` Base.convert(::Type{T}, ::StaticInt{N}) where {T<:Number,N} = convert(T, N) ```
…ng#41877) These get invalidated by loading Static.jl, specifically the method ``` Base.convert(::Type{T}, ::StaticInt{N}) where {T<:Number,N} = convert(T, N) ```
These get invalidated by loading Static.jl, specifically the method
CC @ChrisRackauckas