[JuliaSyntax] Add type-asserts to the results of parse_brackets()
#60476
Conversation
| is_paren_call=is_paren_call, | ||
| is_block=!is_paren_call && num_semis > 0) | ||
| end | ||
| end::NamedTuple{(:needs_parameters, :is_paren_call, :is_block, :delim_flags), Tuple{Bool, Bool, Bool, RawFlags}} |
There was a problem hiding this comment.
Possibly neater?
| end::NamedTuple{(:needs_parameters, :is_paren_call, :is_block, :delim_flags), Tuple{Bool, Bool, Bool, RawFlags}} | |
| end | |
| opts::NamedTuple{(:needs_parameters, :is_paren_call, :is_block, :delim_flags), Tuple{Bool, Bool, Bool, RawFlags}} |
There was a problem hiding this comment.
I actually didn't realize that was possible 👀 Does a type-assert after the variable initialization have the same effect? I think I have a mild preference for the current version because that's clearer to me.
There was a problem hiding this comment.
Type-inference knows that opts is constrained by the type-assert everywhere "downstream" of this, so this should be effectively the same in practice yeah
(no opinion from me w.r.t. style)
There was a problem hiding this comment.
yup what Cody said. To expand: type inference follows the data flow according to program control flow. Unlike in some languages, the type assert opts::T is not a global statement about the binding opts. It just asserts the type of the value bound to opts is T at that one point in the program and inference can follow the program dataflow from there.
Style-wise I think it makes slight sense to separate performance annotations from the rest of the code as they're not semantically relevant for humans reading the code. But that's nitpicking, I think this is fine and we can just merge it :)
JamesWrigley
added
backport 1.12
|
Same comment as #60403 (comment):
|
|
From #60403 (comment):
|
5 participants
As discussed in JuliaLang/JuliaSyntax.jl#600 and JuliaLang/JuliaSyntax.jl#615. Tested manually on 1.12. Should finally for-realsies fix JuliaLang/JuliaSyntax.jl#600 🤞