Skip to content

[MacrosOnImports][Swiftify] Copy module imports from clang node's module to its Swift macro SourceFile #81859

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hnrklssn
Copy link
Contributor

@hnrklssn hnrklssn commented May 30, 2025

Since the _SwiftifyImport peer macro expansion reuses the function signature of the original imported function, it needs to import the same modules as the module where the original clang node resides, to ensure visibility of any symbols that may appear in the signature.

rdar://151611573

@hnrklssn
Copy link
Contributor Author

hnrklssn commented Jun 4, 2025

@swift-ci please smoke test


// CHECK: import ModuleA
// CHECK-NEXT: import ModuleB
// CHECK-NOT: import
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 I don't fully get why only the module's imports that themselves are top-level modules show up in the dump. It seems to work regardless, but it seems a bit off. Do you know if this is intended?

@hnrklssn hnrklssn force-pushed the swiftify-inherit-imports branch from 7d2431e to e5c5c6e Compare June 4, 2025 04:35
@hnrklssn
Copy link
Contributor Author

hnrklssn commented Jun 4, 2025

@swift-ci please smoke test

@hnrklssn hnrklssn force-pushed the swiftify-inherit-imports branch from e5c5c6e to 229280f Compare June 5, 2025 01:17
@hnrklssn
Copy link
Contributor Author

hnrklssn commented Jun 5, 2025

@swift-ci please smoke test

@hnrklssn hnrklssn requested a review from CodaFi June 5, 2025 01:18
@hnrklssn
Copy link
Contributor Author

hnrklssn commented Jun 5, 2025

@swift-ci please smoke test windows

@hnrklssn
Copy link
Contributor Author

hnrklssn commented Jun 5, 2025

@swift-ci please smoke test

// RUN: %target-swift-ide-test -print-module -module-to-print=ClangIncludesNoExportModule -plugin-path %swift-plugin-dir -I %S/Inputs -source-filename=x -enable-experimental-feature SafeInteropWrappers | %FileCheck %s

// swift-ide-test doesn't currently typecheck the macro expansions, so run the compiler as well
// RUN: %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -o %t/ClangIncludesNoExport.swiftmodule -I %S/Inputs -enable-experimental-feature SafeInteropWrappers %s
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 it would be nice to have a run line with C++ interop enabled just in case. Sometimes there are subtle differences between the two modes.

@fahadnayyar
Copy link
Contributor

@swift-ci please smoke test

hnrklssn added 3 commits June 7, 2025 14:21
When macros like _SwiftifyImport are added to a wrapper module for a
clang module, they may need to refer to symbols declared in another
clang module that the wrapped module imports (e.g. because they are used
in the original signature). This adds all the imported clang modules as
implicit imports to the wrapper module.

rdar://151611573
@hnrklssn hnrklssn force-pushed the swiftify-inherit-imports branch from 1b8ba84 to 7c794aa Compare June 7, 2025 21:56
@hnrklssn hnrklssn requested a review from xymus as a code owner June 7, 2025 21:56
@hnrklssn
Copy link
Contributor Author

hnrklssn commented Jun 7, 2025

@swift-ci please smoke test macos

@hnrklssn hnrklssn force-pushed the swiftify-inherit-imports branch from 7c794aa to 726d7b3 Compare June 8, 2025 01:04
@hnrklssn
Copy link
Contributor Author

hnrklssn commented Jun 8, 2025

@swift-ci please smoke test macos

@hnrklssn hnrklssn force-pushed the swiftify-inherit-imports branch from 726d7b3 to b3c5489 Compare June 8, 2025 04:27
@hnrklssn
Copy link
Contributor Author

hnrklssn commented Jun 8, 2025

@swift-ci please smoke test macos

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.

3 participants