- 
          
- 
                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 <[email protected]>
| superseeded by #39395 | 
Inspired by #35710 Co-authored-by: Stephan Hilb <[email protected]>
Inspired by #35710 Co-authored-by: Stephan Hilb <[email protected]>
Inspired by JuliaLang#35710 Co-authored-by: Stephan Hilb <[email protected]>
Inspired by JuliaLang#35710 Co-authored-by: Stephan Hilb <[email protected]>
Inspired by JuliaLang#35710 Co-authored-by: Stephan Hilb <[email protected]>
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: