Skip to content

Infer existential element/key/value types for collection literals as a fallback #4061

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

DougGregor
Copy link
Member

What's in this pull request?

The id-as-Any work regressed cases where Swift code could specify
heterogeneous collection literals, e.g.,:

var states: [String: Any] = [
  "California": [
    "population": 37_000_000,
    "cities": ["Los Angeles", "San Diego", "San Jose"],
  ],
  "Oregon": [
    "population": 4_000_000,
    "cities": ["Portland", "Salem", "Eugene"],
  ]
]

Prior to this, the code worked (when Foundation was imported) because
we'd end up with literals of type [NSObject : AnyObject].

The new defaulting rule says that the element type of an array literal
and the key/value types of a dictionary literal can be defaulted if no
stronger type can be inferred. The default type is:

  • Any, for the element type of an array literal or the value type of a dictionary literal, or
  • AnyHashable, for the key type of a dictionary literal.

The latter is intended to compose with implicit conversions to
AnyHashable, so the most-general inferred dictionary type is
[AnyHashable : Any] and will work for any plausible dictionary
literal.

To prevent this inference from diluting types too greatly, we don't
allow this inference in "top-level" expressions, e.g.,:

let d = ["a" : 1, "b" : "two"]

will produce an error because it's a heterogeneous dictionary literal
at the top level. One should annotate this with, e.g.,:

let d = ["a" : 1, "b" : "two"] as [String : Any]

However, we do permit heterogeneous collections in nested positions,
to support cases like the original motivating example.

Resolved bug number: (rdar://problem/27661580)


Before merging this pull request to apple/swift repository:

  • Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform Comment
All supported platforms @swift-ci Please smoke test
All supported platforms @swift-ci Please smoke test and merge
OS X platform @swift-ci Please smoke test OS X platform
Linux platform @swift-ci Please smoke test Linux platform

A smoke test on macOS does the following:

  1. Builds the compiler incrementally.
  2. Builds the standard library only for macOS. Simulator standard libraries and
    device standard libraries are not built.
  3. lldb is not built.
  4. The test and validation-test targets are run only for macOS. The optimized
    version of these tests are not run.

A smoke test on Linux does the following:

  1. Builds the compiler incrementally.
  2. Builds the standard library incrementally.
  3. lldb is built incrementally.
  4. The swift test and validation-test targets are run. The optimized version of these
    tests are not run.
  5. lldb is tested.

Validation Testing

Platform Comment
All supported platforms @swift-ci Please test
All supported platforms @swift-ci Please test and merge
OS X platform @swift-ci Please test OS X platform
OS X platform @swift-ci Please benchmark
Linux platform @swift-ci Please test Linux platform

Lint Testing

Language Comment
Python @swift-ci Please Python lint

Note: Only members of the Apple organization can trigger swift-ci.

DougGregor and others added 8 commits August 5, 2016 16:17
…r possibilities.

Fixes rdar://problem/17444930.

(cherry picked from commit d073375)
Stub out a simplistic 'meet' operation for types that currently only
handles finding the meet of two class types. Use it to optimize the
constraint solver ever so slightly: when we see a type variable whose
constraints state that it is a supertype of two different concrete
types, we attempt to compute their meet and, if we find it, only
produce one potential binding that is the meet itself.

Note that this is an extremely poor implementation of this concept
that is meant to address a specific regression being introduced by the
defaulting of collection literal element types. A real implementation
would, at the very least:

* Implement a proper 'meet' that covers all subtyping in the language.
* Distinguish between "I don't know if there is a meet" and "I am
  absolutely certain that there is no meet".
* Collapse the constraints themselves to reduce the number of
  constraints in the system, rather than just the number of type
  variable bindings we attempt.

(cherry picked from commit 2bcf555)
…ny project.

This is NFC because nothing sets the bit yet, it is just to aid an ongoing
investigation.

(cherry picked from commit 1d21b4e)
(cherry picked from commit 8c2d0c5)
…d 'AnyHashable' for dictionary keys.

The id-as-Any work regressed cases where Swift code could specify
heterogeneous collection literals, e.g.,

    var states: [String: Any] = [
      "California": [
        "population": 37_000_000,
        "cities": ["Los Angeles", "San Diego", "San Jose"],
      ],
      "Oregon": [
        "population": 4_000_000,
        "cities": ["Portland", "Salem", "Eugene"],
      ]
    ]

Prior to this, the code worked (when Foundation was imported) because
we'd end up with literals of type [NSObject : AnyObject].

The new defaulting rule says that the element type of an array literal
and the key/value types of a dictionary literal can be defaulted if no
stronger type can be inferred. The default type is:

  Any, for the element type of an array literal or the value type of a
  dictionary literal, or

  AnyHashable, for the key type of a dictionary literal.

The latter is intended to compose with implicit conversions to
AnyHashable, so the most-general inferred dictionary type is
[AnyHashable : Any] and will work for any plausible dictionary
literal.

To prevent this inference from diluting types too greatly, we don't
allow this inference in "top-level" expressions, e.g.,

  let d = ["a" : 1, "b" : "two"]

will produce an error because it's a heterogeneous dictionary literal
at the top level. One should annotate this with, e.g.,

  let d = ["a" : 1, "b" : "two"] as [String : Any]

However, we do permit heterogeneous collections in nested positions,
to support cases like the original motivating example.

Fixes rdar://problem/27661580.

(cherry picked from commit 22287dd)
…nt expression kinds.

The constraint generator's optimization to eagerly merge type
variables for different keys in a dictionary literal was too eager,
merging the type variables for (e.g.) a string literal and an integer
literal. This prevented us from properly inferring AnyHashable key
types in dictionary literals. Fixes the rest of rdar://problem/27661580.

(cherry picked from commit 4a569f3)
@DougGregor
Copy link
Member Author

@jckarter please review

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor DougGregor added this to the Swift 3.0 milestone Aug 5, 2016
@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

actually, maybe @rudkx would be a better candidate to review these type checker changes

@shahmishal
Copy link
Member

@swift-ci please test linux

@rudkx
Copy link
Contributor

rudkx commented Aug 6, 2016

LGTM, merge away!

@tkremenek tkremenek merged commit c102e91 into swiftlang:swift-3.0-branch Aug 6, 2016
@DougGregor DougGregor deleted the existential-collection-literal-inference-3.0 branch August 7, 2016 03:22
@jckarter
Copy link
Contributor

jckarter commented Aug 8, 2016

@DougGregor Is the defaulting independent for key and value types? If we have homogeneous keys but heterogeneous values, would we still infer e.g. [String: Any] or fall straight up to [AnyHashable: Any]? I suspect that the [String: Any] case comes up a lot because of JSON.

@rudkx
Copy link
Contributor

rudkx commented Aug 8, 2016

@jckarter We infer ["hi" : 1, "bye" : "two"] as [String : Any] and ["hi" : 1, 2 : "two"] as [AnyHashable : Any]

I'll note that given the not-at-top-level rule, we force the user to write the explicit type in both of these cases.

@jckarter
Copy link
Contributor

jckarter commented Aug 8, 2016

Makes sense, @rudkx, that's what I'd expect. Thanks.

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

Successfully merging this pull request may close these issues.

6 participants