Skip to content

[SymbolGraphGen] don't use Clang modules in the "only re-export public symbols" check #66610

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
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/SymbolGraphGen/SymbolGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,10 @@ bool SymbolGraph::isImplicitlyPrivate(const Decl *D,

// Symbols from exported-imported modules should only be included if they
// were originally public.
if (Walker.isFromExportedImportedModule(D) &&
// We force compiler-equality here to ensure that the presence of an underlying
// Clang module does not prevent internal Swift symbols from being emitted when
// MinimumAccessLevel is set to `internal` or below.
if (Walker.isFromExportedImportedModule(D, /*countUnderlyingClangModule*/false) &&
VD->getFormalAccess() < AccessLevel::Public) {
return true;
}
Expand Down
23 changes: 16 additions & 7 deletions lib/SymbolGraphGen/SymbolGraphASTWalker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,17 @@ namespace {
///
/// This does a by-name comparison to consider a module's underlying Clang module to be equivalent
/// to the wrapping module of the same name.
bool areModulesEqual(const ModuleDecl *lhs, const ModuleDecl *rhs) {
return lhs->getNameStr() == rhs->getNameStr();
///
/// If the `isClangEqual` argument is set to `false`, the modules must also be from the same
/// compiler, i.e. a Swift module and its underlying Clang module would be considered not equal.
bool areModulesEqual(const ModuleDecl *lhs, const ModuleDecl *rhs, bool isClangEqual = true) {
if (lhs->getNameStr() != rhs->getNameStr())
return false;

if (!isClangEqual && (lhs->isNonSwiftModule() != rhs->isNonSwiftModule()))
return false;

return true;
}

} // anonymous namespace
Expand Down Expand Up @@ -303,9 +312,9 @@ bool SymbolGraphASTWalker::isConsideredExportedImported(const Decl *D) const {
return false;
}

bool SymbolGraphASTWalker::isFromExportedImportedModule(const Decl* D) const {
bool SymbolGraphASTWalker::isFromExportedImportedModule(const Decl* D, bool countUnderlyingClangModule) const {
auto *M = D->getModuleContext();
return isQualifiedExportedImport(D) || isExportedImportedModule(M);
return isQualifiedExportedImport(D) || isExportedImportedModule(M, countUnderlyingClangModule);
}

bool SymbolGraphASTWalker::isQualifiedExportedImport(const Decl *D) const {
Expand All @@ -314,9 +323,9 @@ bool SymbolGraphASTWalker::isQualifiedExportedImport(const Decl *D) const {
});
}

bool SymbolGraphASTWalker::isExportedImportedModule(const ModuleDecl *M) const {
return llvm::any_of(ExportedImportedModules, [&M](const auto *MD) {
return areModulesEqual(M, MD->getModuleContext());
bool SymbolGraphASTWalker::isExportedImportedModule(const ModuleDecl *M, bool countUnderlyingClangModule) const {
return llvm::any_of(ExportedImportedModules, [&M, countUnderlyingClangModule](const auto *MD) {
return areModulesEqual(M, MD->getModuleContext(), /*isClangEqual*/countUnderlyingClangModule);
});
}

Expand Down
10 changes: 8 additions & 2 deletions lib/SymbolGraphGen/SymbolGraphASTWalker.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,19 @@ struct SymbolGraphASTWalker : public SourceEntityWalker {
virtual bool isConsideredExportedImported(const Decl *D) const;

/// Returns whether the given declaration comes from an `@_exported import` module.
virtual bool isFromExportedImportedModule(const Decl *D) const;
///
/// If `countUnderlyingClangModule` is `false`, decls from Clang modules will not be considered
/// re-exported unless the Clang module was itself directly re-exported.
virtual bool isFromExportedImportedModule(const Decl *D, bool countUnderlyingClangModule = true) const;

/// Returns whether the given declaration was imported via an `@_exported import <type>` declaration.
virtual bool isQualifiedExportedImport(const Decl *D) const;

/// Returns whether the given module is an `@_exported import` module.
virtual bool isExportedImportedModule(const ModuleDecl *M) const;
///
/// If `countUnderlyingClangModule` is `false`, Clang modules will not be considered re-exported
/// unless the Clang module itself was directly re-exported.
virtual bool isExportedImportedModule(const ModuleDecl *M, bool countUnderlyingClangModule = true) const;

/// Returns whether the given module is the main module, or is an `@_exported import` module.
virtual bool isOurModule(const ModuleDecl *M) const;
Expand Down
20 changes: 20 additions & 0 deletions test/SymbolGraph/ClangImporter/MinimumAccessLevel.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: %empty-directory(%t)
// RUN: cp -r %S/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework %t
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-objc-interop -emit-module-path %t/EmitWhileBuilding.framework/Modules/EmitWhileBuilding.swiftmodule/%target-swiftmodule-name -import-underlying-module -F %t -module-name EmitWhileBuilding -disable-objc-attr-requires-foundation-module %s %S/Inputs/EmitWhileBuilding/Extra.swift -emit-symbol-graph -emit-symbol-graph-dir %t -symbol-graph-minimum-access-level internal
// RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.symbols.json
// RUN: %{python} -c 'import os.path; import sys; sys.exit(1 if os.path.exists(sys.argv[1]) else 0)' %t/EmitWhileBuilding@EmitWhileBuilding.symbols.json

// RUN: %target-swift-symbolgraph-extract -sdk %clang-importer-sdk -module-name EmitWhileBuilding -F %t -output-dir %t -pretty-print -v -minimum-access-level internal
// RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.symbols.json
// RUN: %{python} -c 'import os.path; import sys; sys.exit(1 if os.path.exists(sys.argv[1]) else 0)' %t/EmitWhileBuilding@EmitWhileBuilding.symbols.json

// REQUIRES: objc_interop

// Ensure that having an underlying Clang module does not override the
// `-symbol-graph-minimum-access-level` flag (rdar://110399757)

// CHECK: "s:17EmitWhileBuilding9innerFuncSSyF"

internal func innerFunc() -> String { "sup" }

public func someFunc() -> String { innerFunc() }