Skip to content

Add support for textual imports to -emit-objc-header #60971

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
Nov 30, 2022
Merged

Add support for textual imports to -emit-objc-header #60971

merged 1 commit into from
Nov 30, 2022

Conversation

NuriAmari
Copy link
Contributor

Currently headers produced with -emit-objc-header / -emit-objc-header-path produce headers that include modular imports. If the consumer wishes to operate without modules enabled, these headers cannot be used. This patch introduces a new flag (-emit-objc-header-textually) that when enabled attempts to translate each modular import included in such a header with a set of equivalent textual imports.

@NuriAmari NuriAmari self-assigned this Sep 6, 2022
@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

NuriAmari
NuriAmari commented Sep 6, 2022
View reviewed changes
@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

NuriAmari
NuriAmari commented Sep 7, 2022
View reviewed changes
NuriAmari
NuriAmari commented Sep 7, 2022
View reviewed changes
@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari NuriAmari changed the title Add support for textual includes to -emit-objc-header Add support for textual imports to -emit-objc-header Sep 7, 2022
NuriAmari
NuriAmari commented Sep 7, 2022
View reviewed changes
@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari NuriAmari marked this pull request as ready for review September 7, 2022 17:26
@hyp
Copy link
Contributor

hyp commented Sep 9, 2022

Can this be on by default in the #ifdef block that's going to be active when modules are not enabled?

@NuriAmari
Copy link
Contributor Author

Can this be on by default in the #ifdef block that's going to be active when modules are not enabled?

That makes sense to me. I've got some more changes coming, going to redraft.

@NuriAmari NuriAmari marked this pull request as draft September 9, 2022 15:22
@hyp
Copy link
Contributor

hyp commented Sep 9, 2022

Can this be on by default in the #ifdef block that's going to be active when modules are not enabled?

That makes sense to me. I've got some more changes coming, going to redraft.

Perfect, cheers! This is going to help with C++ interop too, as it would allow us to reinclude C++ headers imported into Swift that we need in the generated C++ header.

@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari NuriAmari marked this pull request as ready for review October 21, 2022 23:48
@NuriAmari NuriAmari requested a review from hyp October 21, 2022 23:59
@NuriAmari NuriAmari requested a review from beccadax October 25, 2022 16:25
hyp
hyp reviewed Oct 26, 2022
View reviewed changes
@hyp
Copy link
Contributor

hyp commented Oct 26, 2022

@swift-ci please test source compatibility

@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari
Copy link
Contributor Author

@swift-ci please test source compatibility

@NuriAmari
Copy link
Contributor Author

@swift-ci Smoke test macOS Platform

@NuriAmari
Copy link
Contributor Author

@swift-ci Please smoke test

@NuriAmari
Copy link
Contributor Author

@swift-ci please test source compatibility

@NuriAmari NuriAmari requested a review from hyp November 15, 2022 18:22
@NuriAmari
Copy link
Contributor Author

@swift-ci please test

@hyp
Copy link
Contributor

hyp commented Nov 18, 2022

@swift-ci please test macOS platform

@hyp
Copy link
Contributor

hyp commented Nov 18, 2022

Seems to fail on macOS:

PrintAsObjC/emit-clang-header-nonmodular-includes.swift

@NuriAmari
Copy link
Contributor Author

Yeah, just saw earlier, will look into it. It only failed on iphonesimulator-x86_64 iirc, so I'm thinking maybe an SDK difference, but I'll update once I figure it out. Thanks!

hyp
hyp reviewed Nov 18, 2022
View reviewed changes
@NuriAmari
Add support for textual imports to -emit-objc-header
89a01dd 
Currently headers produced with `-emit-objc-header` /
`-emit-objc-header-path` produce headers that include modular imports.
If the consumer wishes to operate without modules enabled, these headers
cannot be used. This patch introduces a new flag
(`-emit-clang-header-nonmodular-includes`) that when enabled
attempts to argument each modular import included in such a header with
a set of equivalent textual imports.
@NuriAmari
Copy link
Contributor Author

@swift-ci please test

@NuriAmari NuriAmari requested a review from hyp November 20, 2022 17:26
@hyp
Copy link
Contributor

hyp commented Nov 29, 2022

@swift-ci please test source compatibility

@NuriAmari NuriAmari merged commit 91fd5f8 into swiftlang:main Nov 30, 2022
@etcwilde etcwilde mentioned this pull request Dec 2, 2022
Merged
This was referenced Dec 3, 2022
Closed
Merged
@AZero13 AZero13 mentioned this pull request Feb 14, 2023
Closed
AZero13 added a commit to AZero13/swift that referenced this pull request Feb 14, 2023
@AZero13
Resolve FIXME
c72cf41 
swiftlang#60971 has landed, so we can remove the temporary workarounds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hyp hyp hyp approved these changes

@beccadax beccadax Awaiting requested review from beccadax

@zoecarver zoecarver Awaiting requested review from zoecarver zoecarver is a code owner

@egorzhdan egorzhdan Awaiting requested review from egorzhdan egorzhdan is a code owner

Assignees

@NuriAmari NuriAmari

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NuriAmari @hyp