Skip to content

Conversation

slavapestov
Copy link
Contributor

No description provided.

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov slavapestov requested review from DougGregor and rudkx April 28, 2018 10:04
@slavapestov slavapestov force-pushed the kill-shallow-checking branch from 917d734 to e164ed2 Compare April 28, 2018 10:53
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I was wondering if shallow really necessary...

@rudkx
Copy link
Contributor

rudkx commented Apr 28, 2018

Nice cleanup!

@slavapestov slavapestov force-pushed the kill-shallow-checking branch from e164ed2 to 7bfe081 Compare April 30, 2018 10:34
…CheckExpressionShallow()

Unfortunately, the new implementation is more complex for three reasons:

- SubstitutionMap -vs- SubstitutionList is awkward to work with.

- CallExpr takes a single TupleExpr argument, not a list of arguments.

- The interpolation protocol has a weird behavior. The type checker would
  pick more specific concrete overloads of init(stringInterpolationSegment:)
  on String, even though they're not protocol requirements, and we have tests
  that rely on this behavior.

The first two will be addressed eventually. The latter is contingent on
a redesign of the whole interpolation protocol, which would be a nice
thing to have before ABI stability.
@slavapestov slavapestov force-pushed the kill-shallow-checking branch from 7bfe081 to 3ff0ab9 Compare May 2, 2018 04:28
@slavapestov
Copy link
Contributor Author

I’ve merged everything here except for the last usage which was in string interpolation. That still needs a bit of work to fix a diagnostics regression.

@rudkx
Copy link
Contributor

rudkx commented May 21, 2018

@slavapestov Is this worth keeping open until you take a look at the string interpolation usage?

@slavapestov slavapestov closed this Aug 6, 2018
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.

3 participants