Skip to content

Should we do bottom-up disambiguation of set/map literals with spreads? #167

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

Closed
munificent opened this issue Jan 9, 2019 · 2 comments
Closed

Comments

@munificent
Copy link
Member

This is syntactically ambiguous:

{...a}

It could be a map or set. Right now, the proposal follows the set literal proposal and says the context type can be used to disambiguate:

Set s = {...a}; // Set
Map m = {...}; // Map

It makes sense for the set literal proposal to only use the context type to disambiguate empty literals, because it's empty. What else could you use? But it seems weird for spreads:

var list = [1, 2, 3];
var set = {...list};

As it's proposed, this will generate a type error here. There is no context type, so the {...list} is inferred as a map literal. Then you get a type error trying to spread a list into a map literal.

This feels pretty bad to me. Should we also allow bottom-up inference if there isn't a context type? Would doing so cause any problems I'm not foreseeing? If not, I'm happy to update the proposal.

cc @lrhn @leafpetersen

@lrhn
Copy link
Member

lrhn commented Jan 9, 2019

We knew the rules would have to be generalized for spreads/if/for elements.

For something that is syntactically indistinguishable, we should use types to determine the meaning. Then all following steps can use the static type of the expression to see what the choice was.

I would first use context type if available.
If the basetype of the context type is a subtype of Iterable<Object> (and it's not also a map, as usual), then let T be the type of Iterable that it actually implements.
Then it's a set literal, all sub-parts are elements (not entries), and the context type of each element is T.

If not, it's a map literal and each sub-part is an entry. For map entries, we'll probably have to talk about "key context type" and "value context type" for the map entries. We'll need a new syntactic category for map entries, because we now have sub-entries in if/for constructs.

For a non-empty set-or-map literal and the context type is not available, I'd analyzea the individual element-or-entry expressions, recursively, and give them a vote towards either set or map.

For ... p, find the static type of p with no type context. If that static type is a subtype of Iterable<Object> and not a subtype of Map<Object,Object>, then vote set.
Otherwise if e has a static type that is a subtype of Map<Object, Object>, then it vote map.
If neither, then it doesn't vote either way. (That could mean that it is neither, in which case we might get a decision from some other elements, and then introduce a dynamic downcast, or both, and then we'll do whichever the other parts decide).

For if (e1) p1, analyze p1.
For if (e1) p1 else p2 analyze p1 and p2.
For for (... in e1) p1, analyze p1.
For expression vote set (in case we just collapse set/map literals completely in the grammar and define what it really is using prose).
For expression: expression vote map.

If there is not at least one vote, or there are votes for both set and map, it's a compile-time error, otherwise pick the one that is voted for.

One thing to consider is whether we can make ... {} provide no vote. That would make

var s = {... {}, ...{1}};

successful, and not fail because {} is just seen as a map.
It would also make {...{}} fail, which might be surprising since {} is normally a map.

@leafpetersen
Copy link
Member

We decided to do this, specification for how it is done will be forthcoming shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants