Finish up SE-0096 #3888

CodaFi · 2016-07-30T09:04:11Z

What's in this pull request?

dynamicType is no longer a keyword. There is, however, a diagnostic that will aid migration so usage of dynamicType as an identifier for a static variable is still banned. The parser is also strict about the kind of spelling it will accept for type(of:) and will backtrack if it cannot parse such an expression so we don't step on user's toes.

When this becomes a standard library primitive all of this code can get ripped up.

Resolved bug number: (SR-)

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:

Builds the compiler incrementally.
Builds the standard library only for macOS. Simulator standard libraries and
device standard libraries are not built.
lldb is not built.
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:

Builds the compiler incrementally.
Builds the standard library incrementally.
lldb is built incrementally.
The swift test and validation-test targets are run. The optimized version of these
tests are not run.
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.

CodaFi · 2016-07-30T09:05:43Z

@swift-ci please smoke test.

CodaFi · 2016-07-30T09:11:07Z

test/Parse/type_expr.swift

Will fix this once I get a CI status for this build.

CodaFi · 2016-07-30T09:13:25Z

@jtbandes Thank you for alerting me of the user shadowing of type case. I've relaxed the restriction as much as I can but the loss of diagnostics for such simple cases as in the last patch is troubling. Perhaps this issue can be raised on Swift-evolution.

rintaro · 2016-07-30T09:20:22Z

lib/Parse/ParseExpr.cpp

Does this mean: x.dynamicType doesn't form DynamicTypeExpr any more?

Since sizeof(x.dynamicType) used to be commonly used,
I want to provide fix-it for it in #3883, that relies on DynamicTypeExpr.
Second round fix-it is probably OK:
sizeof(x.dynamicType) => sizeof(type(of: x)) => MemoryLayout<TypeOfX>.size.

Anyway, if so, I think I should adjust #3883 code.

x.dynamicType is removed from the language. type(of: x) forms a DynamicTypeExpr.

If it causes you too much pain it can probably be removed. Much as I think it's nice to have this migration aid, the user will receive a "no such member" error even if it isn't here which should be hint enough that dynamicType is gone. A special cased note can be added to Sema to diagnose this too.

IMO, we should parse and build AST as much as possible.
I tried this PR locally:

func foo<T>(_: T.Type) {} let x = 1 foo(x.dynamicType)

result:

test3888.swift:3:6: error: '.dynamicType' is deprecated. Use 'type(of: ...)' instead foo(x.dynamicType) ^~~~~~~~~~~~ type(of: ) test3888.swift:3:1: error: cannot invoke 'foo' with an argument list of type '(Int)' foo(x.dynamicType) ^ test3888.swift:3:1: note: expected an argument list of type '(T.Type)' foo(x.dynamicType) ^

In this case, I think, we should not emit the second error.
Since x.dynamicType doesn't form DynamicTypeExpr, but just UnresolvedDeclRefExpr of x, TypeChecker cannot know the intended type of x.dynamicType.

Or, could you stop consuming the token here, and remove continue; at the next line, so that it fall back to UnresolvedDotExpr?
That way, result:

test3888.swift:3:6: error: '.dynamicType' is deprecated. Use 'type(of: ...)' instead foo(x.dynamicType) ^~~~~~~~~~~~ type(of: ) test3888.swift:3:5: error: value of type 'Int' has no member 'dynamicType' foo(x.dynamicType) ^ ~~~~~~~~~~~

That's a fantastic idea. It now falls through to that. Hope that helps your patch

CodaFi · 2016-07-30T09:55:19Z

@swift-ci please smoke test.

Be laxer about the parsing for type(of:) so as not to get in the way of other declarations that may use ‘type’ as their base name.

CodaFi · 2016-07-30T10:27:44Z

@swift-ci please smoke test.

CodaFi · 2016-07-30T10:53:55Z

@swift-ci please smoke test.

jtbandes · 2016-07-30T16:19:09Z

Speaking of shadowing, is there anything preventing someone from writing func type<T>(of value: T) ...?

It seems like it would be ideal to put this function in the standard library, even if its actual implementation is replaced by the compiler via some magical hack. I don't know if that's even feasible, though.

CodaFi · 2016-07-30T19:50:15Z

So even if we could put this in stdlib right now why is shadowing this thing a worthwhile feature to have? What use-case could you have for overloading a language primitive?

jtbandes · 2016-07-30T19:53:57Z

I don't think shadowing would really be a good idea for user code, just that there might be code existing today which is weirdly broken by this. It should either allow shadowing (in which case Swift.type(of:) could disambiguate) or it should warn/error on any declarations which would shadow the real one.

CodaFi · 2016-07-30T20:00:06Z

I'm more concerned about the use case that @rintaro brings up than this. If the user has overloaded type(of:) to return something other than T.Type the typechecker will catch it.

CodaFi · 2016-07-30T20:01:22Z

@swift-ci please smoke test.

CodaFi reviewed Jul 30, 2016
View reviewed changes

test/Parse/type_expr.swift Outdated

Copy link

Contributor Author

CodaFi Jul 30, 2016

Choose a reason for hiding this comment

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

Will fix this once I get a CI status for this build.

rintaro reviewed Jul 30, 2016
View reviewed changes

CodaFi force-pushed the some-type-of-wei branch from 2c2655f to 8e747af Compare July 30, 2016 09:51

CodaFi added 2 commits July 30, 2016 03:24

[SE-0096] Remove support for dynamicType member

4e5665a

Fixup diagnostics around type(of:) and dynamicType handling

9c83e72

Be laxer about the parsing for type(of:) so as not to get in the way of other declarations that may use ‘type’ as their base name.

CodaFi force-pushed the some-type-of-wei branch 2 times, most recently from 9eb2300 to d4f0250 Compare July 30, 2016 10:27

Fixup validation tests

e1ae039

CodaFi force-pushed the some-type-of-wei branch from d4f0250 to e1ae039 Compare July 30, 2016 10:53

Improve fallthrough diagnostics

8d23005

CodaFi merged commit 8d4b1cc into swiftlang:master Jul 30, 2016

CodaFi deleted the some-type-of-wei branch July 30, 2016 21:12

Finish up SE-0096 #3888

Finish up SE-0096 #3888

Uh oh!

Conversation

CodaFi commented Jul 30, 2016

What's in this pull request?

Resolved bug number: (SR-)

Uh oh!

CodaFi commented Jul 30, 2016

Uh oh!

CodaFi Jul 30, 2016

Choose a reason for hiding this comment

Uh oh!

CodaFi commented Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rintaro Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rxwei Jul 30, 2016

Choose a reason for hiding this comment

Uh oh!

CodaFi Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CodaFi Jul 30, 2016

Choose a reason for hiding this comment

Uh oh!

CodaFi commented Jul 30, 2016

Uh oh!

CodaFi commented Jul 30, 2016

Uh oh!

CodaFi commented Jul 30, 2016

Uh oh!

jtbandes commented Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CodaFi commented Jul 30, 2016

Uh oh!

jtbandes commented Jul 30, 2016

Uh oh!

CodaFi commented Jul 30, 2016

Uh oh!

CodaFi commented Jul 30, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CodaFi commented Jul 30, 2016 •

edited

Loading

rintaro Jul 30, 2016 •

edited

Loading

CodaFi Jul 30, 2016 •

edited

Loading

rintaro Jul 30, 2016 •

edited

Loading

rintaro Jul 30, 2016 •

edited

Loading

jtbandes commented Jul 30, 2016 •

edited

Loading