Skip to content

[SymbolGraphGen] Handle cxx module imports in swift-symbolgraph-extract #73963

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.

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

Merged
merged 1 commit into from
May 30, 2024

Conversation

rauhul
Copy link
Member

@rauhul rauhul commented May 29, 2024

Updates swift-symbolgraph-extract to parse "-cxx-interoperability-mode"
flags and update the underlying compiler invocation. This fixes a bug
where were are unable to extract the symbol graph from swiftmodules with
transitive cxx modules because we parsed cxx headers as c headers.

rdar://128888548 (Add support for parsing cxx headers)

@rauhul
Copy link
Member Author

rauhul commented May 29, 2024

@swift-ci please build toolchain macOS platform

@bitjammer
Copy link
Contributor

We shouldn't duplicate any parsing or validation logic in swift-symbolgraph-extract if possible. If this were something trivial, I don't think it would matter as much. I believe it's important that the tools that embed the compiler options has synchronized behavior with the frontend itself.

See what CompilerInvocation.cpp:1259 is doing. We could factor that out and use that.

The usage in swift_symbolgraph_extract_main might be along the lines of Invocation.getLangOptions().setCxxInteropFromArgs(Args, Diags) or something like that. It'll mean exposing an API on LangOptions.

Let's see what @QuietMisdreavus thinks.

@bitjammer
Copy link
Contributor

And thank you for contributing to the tool, @rauhul.

@rauhul rauhul force-pushed the cxx-swift-symbolgraph-extract branch from 8d6cd2a to 84fe569 Compare May 29, 2024 20:03
Updates swift-symbolgraph-extract to parse "-cxx-interoperability-mode"
flags and update the underlying compiler invocation. This fixes a bug
where were are unable to extract the symbol graph from swiftmodules with
transitive cxx modules because we parsed cxx headers as c headers.

rdar://128888548 (Add support for parsing cxx headers)
@rauhul rauhul force-pushed the cxx-swift-symbolgraph-extract branch from 84fe569 to 808ccd4 Compare May 29, 2024 20:03
@rauhul rauhul marked this pull request as ready for review May 29, 2024 20:03
@rauhul rauhul requested review from egorzhdan and bitjammer May 29, 2024 20:03
@rauhul rauhul changed the title Add cxx interop support to symbol graph extract [SymbolGraphGen] Handle cxx module imports in swift-symbolgraph-extract May 29, 2024
@rauhul
Copy link
Member Author

rauhul commented May 29, 2024

@swift-ci test

@rauhul
Copy link
Member Author

rauhul commented May 29, 2024

@swift-ci smoke test

rauhul added a commit to swiftlang/swift-package-manager that referenced this pull request May 29, 2024
In order to extract symbol graphs for modules with transitive imports on
cxx modules we need parse headers in cxx mode using the option:
`-cxx-interoperability-mode=default`. This commit updates
`swift-symbolgraph-extract` to pass the option when there is a Cxx
module in the import graph.

This option is a new addition to `swift-symbolgraph-extract` added in
swiftlang/swift#73963.

Prior to this option's introduction, extracting the symbol graph for cxx
dependent modules would fail with an error similar to:
"unknown type name 'namespace'".

With this SwiftPM commit and `swift-symbolgraph-extract` _prior_ to
73963, users will instead see an argument parsing error like: "unknown
option -cxx-interoperability-mode".

With this change and `swift-symbolgraph-extract` 73963, the symbol graph
is extracted properly.
rauhul added a commit to swiftlang/swift-package-manager that referenced this pull request May 29, 2024
In order to extract symbol graphs for modules with transitive imports on
cxx modules we need parse headers in cxx mode using the option:
`-cxx-interoperability-mode=default`. This commit updates
`swift-symbolgraph-extract` to pass the option when there is a Cxx
module in the import graph.

This option is a new addition to `swift-symbolgraph-extract` added in
swiftlang/swift#73963.

Prior to this option's introduction, extracting the symbol graph for cxx
dependent modules would fail with an error similar to:
"unknown type name 'namespace'".

With this SwiftPM commit and `swift-symbolgraph-extract` _prior_ to
73963, users will instead see an argument parsing error like: "unknown
option -cxx-interoperability-mode".

With this change and `swift-symbolgraph-extract` 73963, the symbol graph
is extracted properly.
@@ -319,6 +321,9 @@ namespace swift {
/// to the Swift language version.
version::Version cxxInteropCompatVersion;

void setCxxInteropFromArgs(llvm::opt::ArgList &Args,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ideally this would go somewhere like ArgsToLangOptionsConverter (similarly to the existing ArgsToFrontendOptionsConverter), but I don't think that necessarily has to happen in this patch.

Copy link
Contributor

@daniel-grumberg daniel-grumberg left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for providing this fix!

@rauhul rauhul merged commit 9a28061 into main May 30, 2024
5 checks passed
@rauhul rauhul deleted the cxx-swift-symbolgraph-extract branch May 30, 2024 16:48
rauhul added a commit to swiftlang/swift-package-manager that referenced this pull request May 31, 2024
In order to extract symbol graphs for modules with transitive imports on
cxx modules we need parse headers in cxx mode using the option:
`-cxx-interoperability-mode=default`. This commit updates
`swift-symbolgraph-extract` to pass the option when there is a Cxx
module in the import graph.

This option is a new addition to `swift-symbolgraph-extract` added in
swiftlang/swift#73963.

Prior to this option's introduction, extracting the symbol graph for cxx
dependent modules would fail with an error similar to:
"unknown type name 'namespace'".

With this SwiftPM commit and `swift-symbolgraph-extract` _prior_ to
73963, users will instead see an argument parsing error like: "unknown
option -cxx-interoperability-mode".

With this change and `swift-symbolgraph-extract` 73963, the symbol graph
is extracted properly.
rauhul added a commit to swiftlang/swift-package-manager that referenced this pull request May 31, 2024
In order to extract symbol graphs for modules with transitive imports on
cxx modules we need parse headers in cxx mode using the option:
`-cxx-interoperability-mode=default`. This commit updates
`swift-symbolgraph-extract` to pass the option when there is a Cxx
module in the import graph.

This option is a new addition to `swift-symbolgraph-extract` added in
swiftlang/swift#73963.

Prior to this option's introduction, extracting the symbol graph for cxx
dependent modules would fail with an error similar to:
"unknown type name 'namespace'".

With this SwiftPM commit and `swift-symbolgraph-extract` _prior_ to
73963, users will instead see an argument parsing error like: "unknown
option -cxx-interoperability-mode".

With this change and `swift-symbolgraph-extract` 73963, the symbol graph
is extracted properly.
rauhul added a commit to swiftlang/swift-package-manager that referenced this pull request May 31, 2024
In order to extract symbol graphs for modules with transitive imports on
cxx modules we need parse headers in cxx mode using the option:
`-cxx-interoperability-mode=default`. This commit updates
`swift-symbolgraph-extract` to pass the option when there is a Cxx
module in the import graph.

This option is a new addition to `swift-symbolgraph-extract` added in
swiftlang/swift#73963.

Prior to this option's introduction, extracting the symbol graph for cxx
dependent modules would fail with an error similar to:
"unknown type name 'namespace'".

With this SwiftPM commit and `swift-symbolgraph-extract` _prior_ to
73963, users will instead see an argument parsing error like: "unknown
option -cxx-interoperability-mode".

With this change and `swift-symbolgraph-extract` 73963, the symbol graph
is extracted properly.
rauhul added a commit to swiftlang/swift-package-manager that referenced this pull request Jun 3, 2024
In order to extract symbol graphs for modules with transitive imports on
C++ modules we need parse headers in C++ interop mode using the option:
`-cxx-interoperability-mode=default`. This commit updates
`swift-symbolgraph-extract` to pass the option when there is a C++
module in the import graph.

This option is a new addition to `swift-symbolgraph-extract` added in
swiftlang/swift#73963.

Prior to this option's introduction, extracting the symbol graph for C++
dependent modules would fail with an error similar to: "unknown type
name 'namespace'".

With this SwiftPM commit and `swift-symbolgraph-extract` _prior_ to
swiftlang/swift#73963, users will instead see an argument parsing error
like: "unknown option -cxx-interoperability-mode".

With this change and `swift-symbolgraph-extract` change in
swiftlang/swift#73963, the symbol graph is extracted properly.
rauhul added a commit to swiftlang/swift-package-manager that referenced this pull request Jun 12, 2024
In order to extract symbol graphs for modules with transitive imports on
C++ modules we need parse headers in C++ interop mode using the option:
`-cxx-interoperability-mode=default`. This commit updates
`swift-symbolgraph-extract` to pass the option when there is a C++
module in the import graph.

This option is a new addition to `swift-symbolgraph-extract` added in
swiftlang/swift#73963.

Prior to this option's introduction, extracting the symbol graph for C++
dependent modules would fail with an error similar to: "unknown type
name 'namespace'".

With this SwiftPM commit and `swift-symbolgraph-extract` _prior_ to
swiftlang/swift#73963, users will instead see an argument parsing error
like: "unknown option -cxx-interoperability-mode".

With this change and `swift-symbolgraph-extract` change in
swiftlang/swift#73963, the symbol graph is extracted properly.
rauhul added a commit to swiftlang/swift-package-manager that referenced this pull request Jun 13, 2024
In order to extract symbol graphs for modules with transitive imports on
C++ modules we need parse headers in C++ interop mode using the option:
`-cxx-interoperability-mode=default`. This commit updates
`swift-symbolgraph-extract` to pass the option when there is a C++
module in the import graph.

This option is a new addition to `swift-symbolgraph-extract` added in
swiftlang/swift#73963.

Prior to this option's introduction, extracting the symbol graph for C++
dependent modules would fail with an error similar to: "unknown type
name 'namespace'".

With this SwiftPM commit and `swift-symbolgraph-extract` _prior_ to
swiftlang/swift#73963, users will instead see an argument parsing error
like: "unknown option -cxx-interoperability-mode".

With this change and `swift-symbolgraph-extract` change in
swiftlang/swift#73963, the symbol graph is extracted properly.
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.

4 participants