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

Short-style NamedTuple type can't be used as generic type argument at second position #6014

Closed
straight-shoota opened this issue Apr 26, 2018 · 7 comments
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler

Comments

@straight-shoota
Copy link
Member

The short-style type declaration {x: Int32, y: String} should be equivalent and interchangable with NamedTuple(x: Int32, y: String). But it seems it only works as generic type argument at the first position:

foo : Array({x: Int32, y: String})
bar : Hash({x: Int32, y: String}, String)
baz : Hash(String, {x: Int32, y: String}) # expecting token ')', not '{'
@RX14 RX14 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler labels Apr 26, 2018
@asterite
Copy link
Member

This is a bit complex to implement. For example:

call([] of A, {x: Int32} -> Int32)

The type of the array is obviously a Proc, but look at this:

call([] of A, {x: Int32})

Now the type of the array is A, and then comes a named tuple instance. The parser need to do a big lookahead to determine this, which is bad (the parser tries not to do lookahead).

I think we should either emove the shortcuts for tuples and named tuples. Writing Tuple(Int32, Int32) vs. {Int32, Int32} is not that painful, and same goes with NamedTuple. It will also simplify the language a bit.

Alternatively, just document that this isn't possible, and you'll have to use NamedTuple(...).

@asterite
Copy link
Member

Yet another solution could be to have a different parsing depending on the position. If it's inside generic type arguments, it's obvious that { will refer to a named tuple type. But outside it it might not be that obvious, and maybe a hash literal or named tuple instance literal would be more obvious. But this of course is also confusing... maybe removing the shortcuts is the best thing to do. In any case, these types are not that common to merit a shortcut.

Another idea is, in calls, to assume that in call([] of A, B), will be parsed as [] of A followed by some argument, as proc types are not that common in such generic types. If you do want a proc type, surround with parentheses: call([] of (A, B -> ...)).

@RX14
Copy link
Contributor

RX14 commented Apr 27, 2018

@asterite we've always had different parsing depending on the context. For type arguments, type shortcuts always work, but outside that (method bodies) pretty much never do. For example, var : {x: Int32} has always parsed {x: Int32} as NamedTuple(x: Int32), but var = {x: Int32} has always parsed {x: Int32} as a tuple literal of type NamedTuple(x: Int32.class).

This is a bug report about the "type" context being lost in the second argument of a generic class.

@straight-shoota
Copy link
Member Author

@asterite In your example it's actually the short-style proc type that makes it more complicated because it makes the , ambiguous. Short-style tuple types don't add ambiguity to the type grammar.
It's only that var = {x: Int32} could also be interpreted as var = NamedTuple(x: Int32), but it's clear that this means a named tuple literal.

@asterite
Copy link
Member

So you both propose that we fix it only inside generic arguments (but not after of)? Sounds good to me. In fact, I think I might change the rule a bit to only parse a single type after of, unless you use parenthesis, so lookahead is never needed.

@asterite
Copy link
Member

Oh, actually, it's a bit more complex. For example this:

class Foo
  property block : Int32, Int32 -> Int32
end

Since property just looks like a call, here there's the ambiguity of knowing where what comes after , is a type that eventually forms a proc, or if it's a second argument to property.

So... I guess I'll just improve the parsing for when you are clearly inside generic arguments (basically inside ( of a generic instantiation).

@straight-shoota
Copy link
Member Author

So... I guess I'll just improve the parsing for when you are clearly inside generic arguments (basically inside ( of a generic instantiation).

That's the main issue here 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler
Projects
None yet
Development

No branches or pull requests

3 participants