Revert "Sema: Prefer the submodule import for access-levels diagnostics"

This reverts commit ca8792ca37.

Revert "[ImportResolution] Gracefully handle importing broken clang module"

This reverts commit edb48dff8c.

Revert "[ImportResolution] Don't deduplicate scoped imports"

This reverts commit da96079eb0.

Revert "[ImportResolution] Deduplicate top-level clang modules from import list"

This reverts commit 0ea5677aa1.

Reverting due to namelookup errors.

rdar://164588082
This commit is contained in:
Henrik G. Olsson
2025-11-12 16:07:50 -08:00
parent d886513828
commit ba7b9b0d6b
9 changed files with 48 additions and 160 deletions

View File

@@ -2990,10 +2990,10 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const {
assert(targetModule != getParentModule() &&
"getImportAccessLevel doesn't support checking for a self-import");
/// Order of relevancy of `import` to reach `targetModule` assuming the same
/// visibility level. Lower is better/more authoritative.
/// Order of relevancy of `import` to reach `targetModule`.
/// Lower is better/more authoritative.
auto rateImport = [&](const ImportAccessLevel import) -> int {
auto importedModule = import->module.importedModule->getTopLevelModule();
auto importedModule = import->module.importedModule;
// Prioritize public names:
if (targetModule->getExportAsName() == importedModule->getBaseIdentifier())
@@ -3008,14 +3008,9 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const {
if (targetModule == importedModule->getUnderlyingModuleIfOverlay())
return 3;
// Any import in the sources:
if (import->importLoc.isValid()) {
// Prefer clang submodules to their top level modules:
if (import->module.importedModule != importedModule)
return 4;
return 5;
}
// Any import in the sources.
if (import->importLoc.isValid())
return 4;
return 10;
};
@@ -3025,12 +3020,11 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const {
auto &imports = getASTContext().getImportCache();
ImportAccessLevel restrictiveImport = std::nullopt;
for (auto &import : *Imports) {
auto importedModule = import.module.importedModule->getTopLevelModule();
if ((!restrictiveImport.has_value() ||
import.accessLevel > restrictiveImport->accessLevel ||
(import.accessLevel == restrictiveImport->accessLevel &&
rateImport(import) < rateImport(restrictiveImport))) &&
imports.isImportedBy(targetModule, importedModule)) {
imports.isImportedBy(targetModule, import.module.importedModule)) {
restrictiveImport = import;
}
}

View File

@@ -163,11 +163,6 @@ class ImportResolver final : public DeclVisitor<ImportResolver> {
/// The list of fully bound imports.
SmallVector<AttributedImport<ImportedModule>, 16> boundImports;
/// Set of imported top-level clang modules. We normally don't expect
/// duplicated imports, but importing multiple submodules of the same clang
/// TLM would cause the same TLM to be imported once per submodule.
SmallPtrSet<const ModuleDecl*, 16> seenClangTLMs;
/// All imported modules which should be considered when cross-importing.
/// This is basically the transitive import graph, but with only top-level
/// modules and without reexports from Objective-C modules.
@@ -347,11 +342,6 @@ void swift::performImportResolutionForClangMacroBuffer(
SF.ASTStage = SourceFile::ImportsResolved;
}
static bool isSubmodule(const ModuleDecl* M) {
auto clangMod = M->findUnderlyingClangModule();
return clangMod && clangMod->Parent;
}
//===----------------------------------------------------------------------===//
// MARK: Import handling generally
//===----------------------------------------------------------------------===//
@@ -419,23 +409,14 @@ void ImportResolver::bindImport(UnboundImport &&I) {
I.validateOptions(topLevelModule, SF);
auto alreadyImportedTLM = [ID,this](const ModuleDecl *MD) {
ASSERT(!isSubmodule(MD));
// Scoped imports don't import all symbols from the module, so a scoped
// import does not count the module as imported
if (ID && isScopedImportKind(ID.get()->getImportKind()))
return false;
return !seenClangTLMs.insert(MD).second;
};
if (!M->isNonSwiftModule() || topLevelModule != M || !alreadyImportedTLM(M)) {
if (topLevelModule && topLevelModule != M) {
// If we have distinct submodule and top-level module, add both.
addImport(I, M);
addImport(I, topLevelModule.get());
}
else {
// Add only the import itself.
addImport(I, M);
if (topLevelModule && topLevelModule != M &&
!alreadyImportedTLM(topLevelModule.get())) {
// If we have distinct submodule and top-level module, add both.
// Importing the submodule ensures that it gets loaded, but the decls
// are imported to the TLM, so import that for visibility.
addImport(I, topLevelModule.get());
}
}
crossImport(M, I);
@@ -1655,6 +1636,11 @@ void ImportResolver::findCrossImports(
}
}
static bool isSubmodule(ModuleDecl* M) {
auto clangMod = M->findUnderlyingClangModule();
return clangMod && clangMod->Parent;
}
void ImportResolver::addCrossImportableModules(
AttributedImport<ImportedModule> importDesc) {
// FIXME: namelookup::getAllImports() doesn't quite do what we need (mainly

View File

@@ -167,8 +167,7 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
ModuleDecl *importedVia = attributedImport.module.importedModule,
*sourceModule = D->getModuleContext();
ctx.Diags.diagnose(loc, diag::module_api_import_aliases, D, importedVia,
sourceModule,
importedVia->getTopLevelModule() == sourceModule);
sourceModule, importedVia == sourceModule);
});
auto ignoredDowngradeToWarning = DowngradeToWarning::No;
@@ -290,8 +289,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
ModuleDecl *importedVia = attributedImport.module.importedModule,
*sourceModule = D->getModuleContext();
ctx.Diags.diagnose(loc, diag::module_api_import, D, importedVia,
sourceModule,
importedVia->getTopLevelModule() == sourceModule,
sourceModule, importedVia == sourceModule,
/*isImplicit*/ false);
}
});
@@ -439,7 +437,7 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
ctx.Diags.diagnose(loc, diag::module_api_import_conformance,
rootConf->getType(), rootConf->getProtocol(),
importedVia, sourceModule,
importedVia->getTopLevelModule() == sourceModule);
importedVia == sourceModule);
});
auto originKind = getDisallowedOriginKind(ext, where);

View File

@@ -2777,7 +2777,7 @@ void swift::recordRequiredImportAccessLevelForDecl(const ValueDecl *decl,
*sourceModule = decl->getModuleContext();
dc->getASTContext().Diags.diagnose(
diagLoc, diag::module_api_import, decl, importedVia, sourceModule,
importedVia->getTopLevelModule() == sourceModule, loc.isInvalid());
importedVia == sourceModule, loc.isInvalid());
});
}

View File

@@ -153,7 +153,9 @@ d1_t b2d(void * _Nonnull __sized_by(size), int size);
// DUMP-NEXT: D1
// DUMP-NEXT: A1
// DUMP-NEXT: C1
// DUMP-NEXT: A1
// DUMP-NEXT: B1
// DUMP-NEXT: A1
// DUMP-NEXT: B2
// DUMP-CXX-NEXT: CxxShim
// DUMP-CXX-NEXT: Cxx
@@ -168,7 +170,9 @@ d1_t b2d(void * _Nonnull __sized_by(size), int size);
// DUMP-NEXT: D1
// DUMP-NEXT: A1
// DUMP-NEXT: C1
// DUMP-NEXT: A1
// DUMP-NEXT: B1
// DUMP-NEXT: A1
// DUMP-NEXT: B2
// DUMP-CXX-NEXT: CxxShim
// DUMP-CXX-NEXT: Cxx
@@ -183,7 +187,9 @@ d1_t b2d(void * _Nonnull __sized_by(size), int size);
// DUMP-NEXT: D1
// DUMP-NEXT: A1
// DUMP-NEXT: C1
// DUMP-NEXT: A1
// DUMP-NEXT: B1
// DUMP-NEXT: A1
// DUMP-NEXT: B2
// DUMP-CXX-NEXT: CxxShim
// DUMP-CXX-NEXT: Cxx

View File

@@ -38,12 +38,17 @@ import A2.B2.C2
// DUMP-NEXT: SwiftOnoneSupport
// DUMP-NEXT: A1
// DUMP-NEXT: B1
// DUMP-NEXT: A1
// DUMP-NEXT: C1
// DUMP-NEXT: A1
// DUMP-NEXT: D1
// DUMP-NEXT: A1
// DUMP-NEXT: E1
// DUMP-NEXT: A1
// DUMP-NEXT: B2
// DUMP-NEXT: A2
// DUMP-NEXT: C2
// DUMP-NEXT: A2
public func callUnsafe(_ p: UnsafeMutableRawPointer) {
let _ = a1(p, 13)
@@ -290,7 +295,10 @@ c2_t b2_2(void * _Nonnull __sized_by(size), int size);
// DUMP-NEXT: D1
// DUMP-NEXT: A1
// DUMP-NEXT: C1
// DUMP-NEXT: A1
// DUMP-NEXT: B1
// DUMP-NEXT: A1
// DUMP-NEXT: A1
// DUMP-CXX-NEXT: CxxShim
// DUMP-CXX-NEXT: Cxx
// DUMP-NEXT: _StringProcessing
@@ -305,7 +313,10 @@ c2_t b2_2(void * _Nonnull __sized_by(size), int size);
// DUMP-NEXT: D1
// DUMP-NEXT: A1
// DUMP-NEXT: C1
// DUMP-NEXT: A1
// DUMP-NEXT: B1
// DUMP-NEXT: A1
// DUMP-NEXT: A1
// DUMP-CXX-NEXT: CxxShim
// DUMP-CXX-NEXT: Cxx
// DUMP-NEXT: _StringProcessing
@@ -333,7 +344,10 @@ e2_t c2(void * _Nonnull __sized_by(size), int size);
// DUMP-NEXT: D1
// DUMP-NEXT: A1
// DUMP-NEXT: C1
// DUMP-NEXT: A1
// DUMP-NEXT: B1
// DUMP-NEXT: A1
// DUMP-NEXT: A1
// DUMP-CXX-NEXT: CxxShim
// DUMP-CXX-NEXT: Cxx
// DUMP-NEXT: _StringProcessing
@@ -357,7 +371,10 @@ e2_t d2(void * _Nonnull __sized_by(size), int size);
// DUMP-NEXT: D1
// DUMP-NEXT: A1
// DUMP-NEXT: C1
// DUMP-NEXT: A1
// DUMP-NEXT: B1
// DUMP-NEXT: A1
// DUMP-NEXT: A1
// DUMP-CXX-NEXT: CxxShim
// DUMP-CXX-NEXT: Cxx
// DUMP-NEXT: _StringProcessing

View File

@@ -6,12 +6,9 @@
// RUN: %target-swift-frontend -verify -verify-additional-file %t%{fs-sep}bar.h -typecheck %t/test.swift -I %t
//--- test.swift
// expected-note@+1{{struct 'Baz' imported as 'private' from 'Bar' here}}
private import Foo.Bar
public import Foo
// expected-error@+2{{function cannot be declared public because its parameter uses a private type}}
// expected-note@+1{{struct 'Baz' is imported by this file as 'private' from 'Bar'}}
public func test(x: Baz) {}
//--- foo.h
@@ -20,7 +17,6 @@ public func test(x: Baz) {}
//--- bar.h
#pragma once
// expected-note@+1{{type declared here}}
struct Baz {};
//--- module.modulemap

View File

@@ -1,109 +0,0 @@
/// The order of imports in sources shouldn't matter for access-levels.
// RUN: %empty-directory(%t)
// RUN: split-file %s %t --leading-lines
/// Build the libraries.
// RUN: %target-swift-frontend -emit-module %t/Lib1.swift -module-name Lib1 -o %t
// RUN: %target-swift-frontend -emit-module %t/Lib2.swift -module-name Lib2 -o %t
/// Test main cases.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t -package-name pkg \
// RUN: -verify -verify-ignore-unrelated
// RUN: %target-swift-frontend -typecheck %t/Client_Clang.swift -I %t \
// RUN: -verify -verify-ignore-unrelated
// RUN: %target-swift-frontend -typecheck %t/Client_Clang.swift -I %t -DINVERT \
// RUN: -verify -verify-ignore-unrelated
// RUN: %target-swift-frontend -typecheck %t/Client_Clang_Submodules.swift -I %t \
// RUN: -verify -verify-ignore-unrelated
// RUN: %target-swift-frontend -typecheck %t/Client_Clang_Submodules.swift -I %t -DINVERT \
// RUN: -verify -verify-ignore-unrelated
// REQUIRES: VENDOR=apple
//--- Lib1.swift
public struct Type1 {}
//--- Lib2.swift
public struct Type2 {}
//--- Client.swift
/// Simple public vs internal.
package import Lib1 // expected-note {{imported 'package' here}}
// expected-note @-1 {{struct 'Type1' imported as 'package' from 'Lib1' here}}
internal import Lib1 // expected-warning {{module 'Lib1' is imported as 'package' from the same file; this 'internal' access level will be ignored}}
/// Simple package vs internal, inverted.
internal import Lib2 // expected-warning {{module 'Lib2' is imported as 'package' from the same file; this 'internal' access level will be ignored}}
package import Lib2 // expected-note {{imported 'package' here}}
// expected-note @-1 {{struct 'Type2' imported as 'package' from 'Lib2' here}}
public func dummyAPI(t1: Type1) {} // expected-error {{function cannot be declared public because its parameter uses a package type}}
// expected-note @-1 {{struct 'Type1' is imported by this file as 'package' from 'Lib1'}}
public func dummyAPI(t2: Type2) {} // expected-error {{function cannot be declared public because its parameter uses a package type}}
// expected-note @-1 {{struct 'Type2' is imported by this file as 'package' from 'Lib2'}}
//--- Client_Clang.swift
#if INVERT
private import ClangLib
#endif
internal import ClangLib2 // expected-note {{struct 'ClangType' imported as 'internal' from 'ClangLib2' here}}
#if !INVERT
private import ClangLib
#endif
public func dummyAPI(t2: ClangType) {}
// expected-error @-1 {{function cannot be declared public because its parameter uses an internal type}}
// expected-note @-2 {{struct 'ClangType' is imported by this file as 'internal' from 'ClangLib2'}}
//--- Client_Clang_Submodules.swift
#if INVERT
private import ClangLib.Sub1
#endif
internal import ClangLib.Sub2 // expected-note {{struct 'SubType' imported as 'internal' from 'Sub2' here}}
#if !INVERT
private import ClangLib.Sub1
#endif
public func dummyAPI(t2: SubType) {}
// expected-error @-1 {{function cannot be declared public because its parameter uses an internal type}}
// expected-note @-2 {{struct 'SubType' is imported by this file as 'internal' from 'Sub2'}}
//--- module.modulemap
module ClangLib {
header "ClangLib1.h"
explicit module Sub1 {
header "Sub1.h"
}
explicit module Sub2 {
header "Sub2.h"
}
}
module ClangLib2 {
header "ClangLib2.h"
export *
}
//--- ClangLib1.h
struct ClangType {};
//--- ClangLib2.h
#include <ClangLib1.h>
//--- Sub1.h
struct SubType {};
//--- Sub2.h
#include "Sub1.h"

View File

@@ -337,8 +337,8 @@ public import ClangSubmoduleUnused.ClangSubmoduleUnsuedSubmodule // expected-war
public import ClangTopModule.ClangTopModuleSubmodule
public func clangUser(a: ClangSimpleType) {} // expected-remark {{struct 'ClangSimpleType' is imported via 'ClangSimple'}}
public func clangUser(a: ClangSubmoduleSubmoduleType) {} // expected-remark {{struct 'ClangSubmoduleSubmoduleType' is imported via 'ClangSubmoduleSubmodule'}}
public func clangUser(a: ClangTopModuleType) {} // expected-remark {{struct 'ClangTopModuleType' is imported via 'ClangTopModuleSubmodule'}}
public func clangUser(a: ClangSubmoduleSubmoduleType) {} // expected-remark {{struct 'ClangSubmoduleSubmoduleType' is imported via 'ClangSubmodule'}}
public func clangUser(a: ClangTopModuleType) {} // expected-remark {{struct 'ClangTopModuleType' is imported via 'ClangTopModule'}}
//--- ClientOfClangReexportedSubmodules.swift
public import ClangReexportedSubmodulePublic.ClangReexportedSubmodulePublicSub