-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
make printing of UnionAll typevars shorter
#35710
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
Conversation
|
This is a great improvement, thanks for working on this. The difficulty is that the short-form syntax only works for one level of nesting. For example this PR gives which is not correct; in that case the current output needs to be used. |
|
The difficulty is that the short-form syntax only works for one level of
nesting.
Thanks, then I assume you also care about the order of gensyms within one level?
I.e. `Pair{<:Any,T}` vs `Pair{T,<:Any}` for gensym typevar `T<:Any`: only one
of those can then be printed as `Pair{<:Any,<:Any}`, since the parser seems to
assign them in fixed order.
This could get a bit ugly. I need to figure out exactly how the parser assigns
the gensyms and how it creates the unionall hierarchy in order to produce the
inverse of that. I'll think about it.
|
|
Yes, that's a good point. Anyway you can probably tell why this hasn't been implemented already :) |
e43f192 to
35962a6
Compare
|
next try:
|
35962a6 to
ea51a06
Compare
|
|
|
Wow, this is really impressive! As far as I can tell it works correctly. Just amazing how difficult this is to implement.
|
FWIW, I like it. |
ea51a06 to
f5998d0
Compare
|
I added some comments explaining the keys. If one was to touch the method printing stuff too, simplifying the logic a bit might be possible, but otherwise there are just too many cornercases making it difficult. Does |
|
Here is a small bug: Similarly this quasi-bug where the input and output forms are swapped: It would be ok to print both as I also think this doesn't need to depend on A future improvement would be to add support for Unions: but that can be done in a later PR. |
f5998d0 to
861438c
Compare
|
Changes:
Since e.g. which is fully reparsable. Parametric type aliases might still be somewhat ugly: But finding the shortest alias representation in general is probably over-the-top, so that is left for the future. |
fixes JuliaLang#34887 typevars in unionall type arguments can now be printed inlined if their type bounds allow to do so (e.g. `<:UB` or `>:LB`) and they are not referenced from elsewhere in the type hierarchy. Before: julia> Ref{<:Any} Ref{var"#s2"} where var"#s2" julia> Ref{<:Ref{<:Ref}} Ref{var"#s1"} where var"#s1"<:(Ref{var"#s2"} where var"#s2"<:Ref) After: julia> Ref{<:Any} Ref julia> Ref{<:Ref{<:Ref}} Ref{<:Ref{<:Ref}}
861438c to
ddd6afb
Compare
Fully agree with both; the first priority is to print everything correctly, and we don't need every possible improvement at once. |
|
I tried to resolve the conflicts but I'm not sure it will work. Let's see what happens. |
|
Ok, |
|
Thanks, I almost forgot about this. struct A{T,S} end
const B{T,S} = A{S,T}
show(B) # previously showed `A{<:Any,T} where T`, now needs to know about `B` to elide typevars when printingThis small example is trivial to fix but the proper solution would need to build the fully recursed typealias representation before printing (another |
|
Yes, you're right. Sorry to do that to you. This PR is a great piece of work. We merged the typealias printing first since in some common cases it reduces the amount of output very drastically. |
Inspired by #35710 Co-authored-by: Stephan Hilb <stephan@ecshi.net>
|
superseeded by #39395 |
Inspired by #35710 Co-authored-by: Stephan Hilb <stephan@ecshi.net>
Inspired by #35710 Co-authored-by: Stephan Hilb <stephan@ecshi.net>
Inspired by JuliaLang#35710 Co-authored-by: Stephan Hilb <stephan@ecshi.net>
Inspired by JuliaLang#35710 Co-authored-by: Stephan Hilb <stephan@ecshi.net>
Inspired by JuliaLang#35710 Co-authored-by: Stephan Hilb <stephan@ecshi.net>
fixes #34887
typevars with gensym symbols will now be printed inlined for
UnionAlltype arguments if their type bounds allow to do so (i.e.<:UBor>:LB).typevars with named symbols are printed as before since they are assumed to carry meaning and thus might be helpful.
Before:
After: