Allow spread element types to disambiguate a set/map literal. #168
Conversation
| 1. If `Map<Null, Null>` is assignable to the literal's context type, then it | ||
| is a map literal. | ||
|
|
||
| 2. Else, if `Set<Null>` is assignable to the literal's context type, then the |
There was a problem hiding this comment.
Note that this is not the criteria that is specified for empty set literals, but there's active discussion about this here: dart-lang/sdk#35431
| 2. Else, if `Set<Null>` is assignable to the literal's context type, then the | ||
| collection is a set literal. | ||
|
|
||
| 3. Else, if it has spread elements and all of the spread element expression |
There was a problem hiding this comment.
I would phrase this differently. I think it's probably better to say that that if there is a non-empty context, you resolve based on the context type (as above, or however we decide). Only if there is no context type do you resolve based on the upwards inference. Otherwise, I think it's guaranteed to be an error anyway, and you're just making things confusing by still trying to use upwards inference.
There was a problem hiding this comment.
Agree. You only need to address the case where there is no context type and no type arguments. The rest are already handled and is unaffected by spreads or comprehensions.
I think we need to rewrite everything around this so that we only have "setOrMapLiteral" as a syntactic category. It's going to be complicated to have:
setLiteral ::= `{` setElements `}`
mapLiteral ::= `{ mapEntries `}`
mapEntry ::= expression `:` expression | `...` expression | `if` `(` expression `)` mapEntry (`else` mapEntry)? | `for` `(` ... `in` expression `)` mapEntry ;
setElement ::= expression | `...` expression | `if` `(` expression `)` setElement (`else` setElement)? | `for` `(` ... `in` expression `)` setElement ;
setOrMapPart ::= | `...` expression | `if` `(` expression `)` setOrMapPart (`else` setOrMapPart)? | `for` `(` ... `in` expression `)` setOrMapPart ;
because they do overlap unless deep-down in the structure there is either an e element or an e1:e2 entry.
So, I'd go for unifying sets and maps, and then use the context type or content parts to determine which one it is. And I wouldn't separate spreads and if/for constructs, they both need the same treatment anyway.
Let me try doing that.
| 3. Else, if it has spread elements and all of the spread element expression | ||
| types are subtypes of `Iterable`, then it is a set literal. | ||
|
|
||
| **TODO: Is dynamic a subtype of Iterable? If so, this needs to be tweaked. |
|
I'm gonna go ahead and close this in favor of the Grand Unified Map/Set doc that's coming. |
This is a stab at addressing #167. Thoughts?