-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[cxx-interop] Add flag to set minimum access level for reverse interop #84816
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -20,6 +20,7 @@ | |||||||||||||||||
#include "PrintSwiftToClangCoreScaffold.h" | ||||||||||||||||||
#include "SwiftToClangInteropContext.h" | ||||||||||||||||||
|
||||||||||||||||||
#include "swift/AST/AttrKind.h" | ||||||||||||||||||
#include "swift/AST/Decl.h" | ||||||||||||||||||
#include "swift/AST/DiagnosticsSema.h" | ||||||||||||||||||
#include "swift/AST/ExistentialLayout.h" | ||||||||||||||||||
|
@@ -1151,35 +1152,41 @@ class ModuleWriter { | |||||||||||||||||
}; | ||||||||||||||||||
} // end anonymous namespace | ||||||||||||||||||
|
||||||||||||||||||
static AccessLevel getRequiredAccess(const ModuleDecl &M) { | ||||||||||||||||||
return M.isExternallyConsumed() ? AccessLevel::Public : AccessLevel::Internal; | ||||||||||||||||||
static AccessLevel getRequiredAccess(const ModuleDecl &M, | ||||||||||||||||||
AccessLevel minAccess) { | ||||||||||||||||||
return M.isExternallyConsumed() ? minAccess : AccessLevel::Internal; | ||||||||||||||||||
Comment on lines
+1155
to
+1157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don’t think I like this behavior. If you explicitly specify an access level on the command line, that ought to override the implicit computation completely, even when
Suggested change
With matching changes upstream of this function to preserve the optionality of |
||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
void swift::printModuleContentsAsObjC( | ||||||||||||||||||
raw_ostream &os, llvm::SmallPtrSetImpl<ImportModuleTy> &imports, | ||||||||||||||||||
ModuleDecl &M, SwiftToClangInteropContext &interopContext) { | ||||||||||||||||||
ModuleDecl &M, SwiftToClangInteropContext &interopContext, | ||||||||||||||||||
AccessLevel minAccess) { | ||||||||||||||||||
llvm::raw_null_ostream prologueOS; | ||||||||||||||||||
llvm::StringSet<> exposedModules; | ||||||||||||||||||
ModuleWriter(os, prologueOS, imports, M, interopContext, getRequiredAccess(M), | ||||||||||||||||||
ModuleWriter(os, prologueOS, imports, M, interopContext, | ||||||||||||||||||
getRequiredAccess(M, minAccess), | ||||||||||||||||||
/*requiresExposedAttribute=*/false, exposedModules, | ||||||||||||||||||
OutputLanguageMode::ObjC) | ||||||||||||||||||
.write(); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
void swift::printModuleContentsAsC( | ||||||||||||||||||
raw_ostream &os, llvm::SmallPtrSetImpl<ImportModuleTy> &imports, | ||||||||||||||||||
ModuleDecl &M, SwiftToClangInteropContext &interopContext) { | ||||||||||||||||||
ModuleDecl &M, SwiftToClangInteropContext &interopContext, | ||||||||||||||||||
AccessLevel minAccess) { | ||||||||||||||||||
llvm::raw_null_ostream prologueOS; | ||||||||||||||||||
llvm::StringSet<> exposedModules; | ||||||||||||||||||
ModuleWriter(os, prologueOS, imports, M, interopContext, getRequiredAccess(M), | ||||||||||||||||||
ModuleWriter(os, prologueOS, imports, M, interopContext, | ||||||||||||||||||
getRequiredAccess(M, minAccess), | ||||||||||||||||||
/*requiresExposedAttribute=*/false, exposedModules, | ||||||||||||||||||
OutputLanguageMode::C) | ||||||||||||||||||
.write(); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
EmittedClangHeaderDependencyInfo swift::printModuleContentsAsCxx( | ||||||||||||||||||
raw_ostream &os, ModuleDecl &M, SwiftToClangInteropContext &interopContext, | ||||||||||||||||||
bool requiresExposedAttribute, llvm::StringSet<> &exposedModules) { | ||||||||||||||||||
AccessLevel minAccess, bool requiresExposedAttribute, | ||||||||||||||||||
llvm::StringSet<> &exposedModules) { | ||||||||||||||||||
std::string moduleContentsBuf; | ||||||||||||||||||
llvm::raw_string_ostream moduleOS{moduleContentsBuf}; | ||||||||||||||||||
std::string modulePrologueBuf; | ||||||||||||||||||
|
@@ -1197,8 +1204,8 @@ EmittedClangHeaderDependencyInfo swift::printModuleContentsAsCxx( | |||||||||||||||||
|
||||||||||||||||||
// FIXME: Use getRequiredAccess once @expose is supported. | ||||||||||||||||||
ModuleWriter writer(moduleOS, prologueOS, info.imports, M, interopContext, | ||||||||||||||||||
AccessLevel::Public, requiresExposedAttribute, | ||||||||||||||||||
exposedModules, OutputLanguageMode::Cxx); | ||||||||||||||||||
minAccess, requiresExposedAttribute, exposedModules, | ||||||||||||||||||
OutputLanguageMode::Cxx); | ||||||||||||||||||
writer.write(); | ||||||||||||||||||
info.dependsOnStandardLibrary = writer.isStdlibRequired(); | ||||||||||||||||||
if (M.isStdlibModule()) { | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
#include "SwiftToClangInteropContext.h" | ||
|
||
#include "swift/AST/ASTContext.h" | ||
#include "swift/AST/AttrKind.h" | ||
#include "swift/AST/Module.h" | ||
#include "swift/AST/PrettyStackTrace.h" | ||
#include "swift/Basic/Assertions.h" | ||
|
@@ -620,7 +621,9 @@ bool swift::printAsClangHeader(raw_ostream &os, ModuleDecl *M, | |
if (M->getASTContext().LangOpts.hasFeature(Feature::CDecl)) { | ||
SmallPtrSet<ImportModuleTy, 8> imports; | ||
llvm::raw_string_ostream cModuleContents{moduleContentsScratch}; | ||
printModuleContentsAsC(cModuleContents, imports, *M, interopContext); | ||
printModuleContentsAsC( | ||
cModuleContents, imports, *M, interopContext, | ||
frontendOpts.ClangHeaderMinAccess.value_or(AccessLevel::Public)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
||
llvm::StringMap<StringRef> exposedModuleHeaderNames; | ||
writeImports(os, imports, *M, bridgingHeader, frontendOpts, | ||
|
@@ -634,7 +637,9 @@ bool swift::printAsClangHeader(raw_ostream &os, ModuleDecl *M, | |
// Objective-C content | ||
SmallPtrSet<ImportModuleTy, 8> imports; | ||
llvm::raw_string_ostream objcModuleContents{moduleContentsScratch}; | ||
printModuleContentsAsObjC(objcModuleContents, imports, *M, interopContext); | ||
printModuleContentsAsObjC( | ||
objcModuleContents, imports, *M, interopContext, | ||
frontendOpts.ClangHeaderMinAccess.value_or(AccessLevel::Public)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
emitObjCConditional(os, [&] { | ||
llvm::StringMap<StringRef> exposedModuleHeaderNames; | ||
writeImports(os, imports, *M, bridgingHeader, frontendOpts, | ||
|
@@ -685,6 +690,7 @@ bool swift::printAsClangHeader(raw_ostream &os, ModuleDecl *M, | |
llvm::raw_string_ostream moduleContents{moduleContentsBuf}; | ||
auto deps = printModuleContentsAsCxx( | ||
moduleContents, *M, interopContext, | ||
frontendOpts.ClangHeaderMinAccess.value_or(AccessLevel::Public), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly here too. |
||
/*requiresExposedAttribute=*/requiresExplicitExpose, exposedModules); | ||
// FIXME: In ObjC++ mode, we do not need to reimport duplicate modules. | ||
llvm::StringMap<StringRef> exposedModuleHeaderNames; | ||
|
@@ -701,9 +707,10 @@ bool swift::printAsClangHeader(raw_ostream &os, ModuleDecl *M, | |
auto macroGuard = computeMacroGuard(M->getASTContext().getStdlibModule()); | ||
os << "#ifndef " << macroGuard << "\n"; | ||
os << "#define " << macroGuard << "\n"; | ||
printModuleContentsAsCxx( | ||
os, *M->getASTContext().getStdlibModule(), interopContext, | ||
/*requiresExposedAttribute=*/true, exposedModules); | ||
printModuleContentsAsCxx(os, *M->getASTContext().getStdlibModule(), | ||
interopContext, AccessLevel::Public, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this fixed as |
||
/*requiresExposedAttribute=*/true, | ||
exposedModules); | ||
os << "#endif // " << macroGuard << "\n"; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// RUN: %empty-directory(%t) | ||
// RUN: %target-swift-frontend %s -module-name Core -typecheck -verify -emit-clang-header-path %t/core.h -clang-header-expose-decls=has-expose-attr -emit-clang-header-min-access public -package-name Core | ||
// RUN: %FileCheck %s --check-prefix CHECK-PUBLIC < %t/core.h | ||
|
||
// RUN: %empty-directory(%t) | ||
// RUN: %target-swift-frontend %s -module-name Core -typecheck -verify -emit-clang-header-path %t/core.h -clang-header-expose-decls=has-expose-attr -emit-clang-header-min-access package -package-name Core | ||
// RUN: %FileCheck %s --check-prefix CHECK-PACKAGE < %t/core.h | ||
|
||
// RUN: %empty-directory(%t) | ||
// RUN: %target-swift-frontend %s -module-name Core -typecheck -verify -emit-clang-header-path %t/core.h -clang-header-expose-decls=has-expose-attr -emit-clang-header-min-access internal -package-name Core | ||
// RUN: %FileCheck %s --check-prefix CHECK-INTERNAL < %t/core.h | ||
|
||
@_expose(Cxx) | ||
public func publicFunc(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
@_expose(Cxx) | ||
package func packageFunc(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
@_expose(Cxx) | ||
internal func internalFunc(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
@_expose(Cxx) | ||
private func privateFunc(_ x: Int) -> Int { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is never emitted, should we warn about sub-internal decls being annotated? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe. This is also used for |
||
return x | ||
} | ||
|
||
// CHECK-PUBLIC-NOT: internalFunc | ||
// CHECK-PUBLIC-NOT: packageFunc | ||
// CHECK-PUBLIC-NOT: privateFunc | ||
// CHECK-PUBLIC: SWIFT_INLINE_THUNK swift::Int publicFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core10publicFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
|
||
// CHECK-PACKAGE-NOT: internalFunc | ||
// CHECK-PACKAGE: SWIFT_INLINE_THUNK swift::Int packageFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core11packageFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-PACKAGE-NOT: privateFunc | ||
// CHECK-PACKAGE: SWIFT_INLINE_THUNK swift::Int publicFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core10publicFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
|
||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int internalFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core12internalFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int packageFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core11packageFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-INTERNAL-NOT: privateFunc | ||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int publicFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core10publicFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
// RUN: %empty-directory(%t) | ||
// RUN: %target-swift-frontend %s -module-name Core -typecheck -verify -emit-clang-header-path %t/core.h -clang-header-expose-decls=all-public -emit-clang-header-min-access public -package-name Core | ||
// RUN: %FileCheck %s --check-prefix CHECK-PUBLIC < %t/core.h | ||
|
||
// RUN: %empty-directory(%t) | ||
// RUN: %target-swift-frontend %s -module-name Core -typecheck -verify -emit-clang-header-path %t/core.h -clang-header-expose-decls=all-public -emit-clang-header-min-access package -package-name Core | ||
// RUN: %FileCheck %s --check-prefix CHECK-PACKAGE < %t/core.h | ||
|
||
// RUN: %empty-directory(%t) | ||
// RUN: %target-swift-frontend %s -module-name Core -typecheck -verify -emit-clang-header-path %t/core.h -clang-header-expose-decls=all-public -emit-clang-header-min-access internal -package-name Core | ||
// RUN: %FileCheck %s --check-prefix CHECK-INTERNAL < %t/core.h | ||
|
||
// RUN: %empty-directory(%t) | ||
// RUN: not %target-swift-frontend %s -module-name Core -typecheck -emit-clang-header-path %t/core.h -clang-header-expose-decls=all-public -emit-clang-header-min-access inernal -package-name Core 2>&1 | %FileCheck %s --check-prefix CHECK-DIAGNOSTIC | ||
|
||
public func publicFunc(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
package func packageFunc(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
internal func internalFunc(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
private func privateFunc(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
public struct S { | ||
public func publicMethod(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
package func packageMethod(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
internal func internalMethod(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
private func privateMethod(_ x: Int) -> Int { | ||
return x | ||
} | ||
|
||
private var x: Int | ||
} | ||
|
||
// CHECK-PUBLIC-NOT: internalFunc | ||
// CHECK-PUBLIC-NOT: packageFunc | ||
// CHECK-PUBLIC-NOT: privateFunc | ||
// CHECK-PUBLIC: SWIFT_INLINE_THUNK swift::Int publicFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core10publicFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-PUBLIC: SWIFT_INLINE_THUNK swift::Int S::publicMethod(swift::Int x) const { | ||
// CHECK-PUBLIC-NOT: packageMethod | ||
// CHECK-PUBLIC-NOT: internalMethod | ||
// CHECK-PUBLIC-NOT: privateMethod | ||
|
||
// CHECK-PACKAGE-NOT: internalFunc | ||
// CHECK-PACKAGE: SWIFT_INLINE_THUNK swift::Int packageFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core11packageFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-PACKAGE-NOT: privateFunc | ||
// CHECK-PACKAGE: SWIFT_INLINE_THUNK swift::Int publicFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core10publicFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-PACKAGE: SWIFT_INLINE_THUNK swift::Int S::publicMethod(swift::Int x) const { | ||
// CHECK-PACKAGE: SWIFT_INLINE_THUNK swift::Int S::packageMethod(swift::Int x) const { | ||
// CHECK-PACKAGE-NOT: internalMethod | ||
// CHECK-PACKAGE-NOT: privateMethod | ||
|
||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int internalFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core12internalFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int packageFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core11packageFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-INTERNAL-NOT: privateFunc | ||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int publicFunc(swift::Int x) noexcept SWIFT_SYMBOL("s:4Core10publicFuncyS2iF") SWIFT_WARN_UNUSED_RESULT { | ||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int S::publicMethod(swift::Int x) const { | ||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int S::packageMethod(swift::Int x) const { | ||
// CHECK-INTERNAL: SWIFT_INLINE_THUNK swift::Int S::internalMethod(swift::Int x) const { | ||
// CHECK-INTERNAL-NOT: privateMethod | ||
|
||
// CHECK-DIAGNOSTIC: error: invalid minimum clang header access level 'inernal'; chose from 'public'|'package'|'internal' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be an explicit spelling for the default behavior? Something like
auto
ordefault
?