Skip to content

[SILGen] Don't SILGen an @objc entry point for a deinit with no body #19472

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 28, 2018
Merged

[SILGen] Don't SILGen an @objc entry point for a deinit with no body #19472

merged 1 commit into from
Sep 28, 2018

Conversation

jrose-apple
Copy link
Contributor

In general we'll want to investigate what we are and aren't SILGen-ing for textual interfaces of resilient modules, but for fragile modules we may as well generate everything we can for potential optimization purposes.

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d6ae8913c150e107d46e3c08bb9f5865b704a762

@jrose-apple
Copy link
Contributor Author

…*sigh* right.

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d6ae8913c150e107d46e3c08bb9f5865b704a762

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d6ae8913c150e107d46e3c08bb9f5865b704a762

slavapestov
slavapestov reviewed Sep 22, 2018
View reviewed changes
Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

Why is this specific to destructors? Does constructor and method emission need similar checks?

@jrose-apple
Copy link
Contributor Author

Hm. We have been testing those with non-@objc entry points, but not with @objc. I'll add that.

(Right now we're mostly trying to get the stdlib and overlays all building successfully, and this one came up in Foundation while the others didn't. But that doesn't mean we shouldn't have tests.)

@jrose-apple
Copy link
Contributor Author

@swift-ci Please smoke test

@jrose-apple
Copy link
Contributor Author

Uh, hm, that's fun. I guess we're emitting unnecessary thunks for constructors too, but it's only a problem on Linux!

@jrose-apple jrose-apple mentioned this pull request Sep 28, 2018
Merged
@jrose-apple
Copy link
Contributor Author

Filed SR-8877 for the Linux failure. I want to get the console output before moving on, so

@swift-ci Please smoke test Linux

@jrose-apple
[SILGen] Don't SILGen an @objc entry point for a deinit with no body
6d4a1f7 
In general we'll want to investigate what we are and aren't SILGen-ing
for textual interfaces of resilient modules, but for fragile modules
we may as well generate everything we can for potential optimization
purposes.
@jrose-apple
Copy link
Contributor Author

Linux CI run on old test file

@swift-ci Please smoke test

@jrose-apple
Copy link
Contributor Author

Ugh, right, that kills the previous job for normal tests. Oh well.

@jrose-apple jrose-apple merged commit c66fcf2 into swiftlang:master Sep 28, 2018
@jrose-apple jrose-apple deleted the no-entry branch September 28, 2018 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavapestov slavapestov slavapestov left review comments

@harlanhaskins harlanhaskins harlanhaskins approved these changes

@jckarter jckarter Awaiting requested review from jckarter

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jrose-apple @swift-ci @slavapestov @harlanhaskins