Skip to content

Conversation

slavapestov
Copy link
Contributor

No description provided.

@slavapestov slavapestov requested a review from DougGregor January 8, 2019 05:20
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility



class C<T> {
dynamic func broken() { } // expected-error{{'dynamic'}}
func broken() { }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DougGregor do you have a test case that demonstrates an actual crash or miscompile? Here we no longer finalize broken() but I'm not convinced that's a problem.

Copy link
Member

@DougGregor DougGregor Jan 8, 2019

Choose a reason for hiding this comment

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

Hmm. I don't have a test case that demonstrates the actual crash; this test case was heavily reduced.

The logic behind my addition of this requestNominalLayout call (in 76eb767) is that when you call a function, you produce a value of that type. IRGen is going to need to be able to lay out that type, so the type checker needs to be able to precompute it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getAnyNominal() is not sufficient, though. You might return a tuple containing nominals, or optionals, etc, and extract fields from there without making further calls.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that it's not sufficient.

@slavapestov slavapestov merged commit fc50e81 into swiftlang:master Jan 8, 2019
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.

2 participants