Skip to content
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

Drop parentheses around -> inside certain comma-separated lists #14506

Merged

Conversation

HertzDevil
Copy link
Contributor

Fixes #14216.

The formatter is not affected, since it will preserve any number of parentheses around a ProcNotation.

@straight-shoota straight-shoota added this to the 1.13.0 milestone Apr 18, 2024
@HertzDevil HertzDevil marked this pull request as draft April 18, 2024 16:06
@HertzDevil HertzDevil marked this pull request as ready for review April 18, 2024 16:27
@HertzDevil HertzDevil removed this from the 1.13.0 milestone Apr 18, 2024
@HertzDevil
Copy link
Contributor Author

HertzDevil commented Apr 18, 2024

I have been thinking about this: if macro interpolation transforms {T} to ::Tuple(T), {x: T} to ::NamedTuple(x: T), T[1] to ::StaticArray(T, 1), and T* to ::Pointer(T), wouldn't it make sense to simply turn all the ->s into ::Procs? Maybe that would be the simplest fix after all?

That leaves unions but IIRC there were still some edge cases making T | U and ::Union(T, U) not semantically equivalent (e.g. #11891).

@straight-shoota
Copy link
Member

Yeah I think that makes sense 👍

@HertzDevil
Copy link
Contributor Author

HertzDevil commented Apr 18, 2024

Well:

lib Lib
  fun foo(x : Int, Int)           # Error: expecting token '->', not ')'
  fun foo(x : Pointer(Void), Int) # Error: expecting token '->', not ')'
  fun foo(x : ::Proc(::Nil), Int) # Error: expecting token '->', not ')'
end

So ::Proc won't help here, but dropping the parentheses does, so let's go forward with this PR anyway. Also the return type of -> being optional makes things a bit complicated regarding #10931 (although -> cannot really be ambiguous as a block restriction).

@straight-shoota
Copy link
Member

This seems awefully complex, but I guess there's no other way to handle the ambiguity issues around -> 😭

@straight-shoota straight-shoota added this to the 1.13.0 milestone Apr 19, 2024
@straight-shoota straight-shoota merged commit 62de01c into crystal-lang:master Apr 20, 2024
60 checks passed
@HertzDevil HertzDevil deleted the bug/procnotation-to_s-paren branch April 20, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ProcNotation followed by anonymous fun parameter breaks on macro interpolation
2 participants