From 8d4e81c2faae308c0f2b95a263d01142b5bbeedf Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Wed, 1 Oct 2025 23:37:47 -0700 Subject: [PATCH 1/2] [ImportResolution] Only disallow explicit `std` imports (allow implicit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When expanding a Swift macro in a clang module where the original clang module imported a submodule in a C++ standard library module other than `std`, e.g. a submodule to `std_core`, this would result in an error. This is because `std_core.math.abs` would be imported as `CxxStdlib.math.abs`, which would later be translated back as `std.math.abs` which doesn’t exist. This changes the mapping to only map `std` to `CxxStdlib`. To prevent errors when importing modules starting with `std_`, this error is moved from the late-stage module import to the earlier processing of `ImportDecl`s. This results in these module names still being forbidden in explicit imports (i.e. naming them in source code), while still being allowed in implicit imports inherited from clang modules. This also fixes a fix-it bug where only the first 3 characters would be selected for replacing with `CxxStdlib` when importing `std_core`. This also fixes a diagnostic bug where aliased modules would refer to the module name in the source code rather than the real module name, and adds a note clarifying the situation. rdar://161795429 rdar://161795673 rdar://161795793 fix non-fatal import error --- include/swift/AST/DiagnosticsSema.def | 2 + lib/ClangImporter/ClangImporter.cpp | 6 +-- lib/Sema/ImportResolution.cpp | 45 +++++++++++++++---- test/ImportResolution/import-alias-fail.swift | 5 +++ test/ImportResolution/import-std-fail.swift | 42 +++++++++++++++++ .../transitive-std-core-import.swift | 33 ++++++++++++++ 6 files changed, 120 insertions(+), 13 deletions(-) create mode 100644 test/ImportResolution/import-alias-fail.swift create mode 100644 test/ImportResolution/import-std-fail.swift create mode 100644 test/Interop/Cxx/swiftify-import/transitive-std-core-import.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index dbee278386036..8cda6b72da572 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -875,6 +875,8 @@ WARNING(sema_import_current_module_with_file,none, (StringRef, Identifier)) ERROR(sema_opening_import,Fatal, "opening import file for module %0: %1", (Identifier, StringRef)) +NOTE(sema_module_aliased,none, + "module name '%0' is aliased to '%1'", (StringRef, StringRef)) REMARK(serialization_skipped_invalid_decl,none, "serialization skipped invalid %kind0", diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 60cc7c87fbd32..b7d8c4671c1ef 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -2415,10 +2415,6 @@ ModuleDecl *ClangImporter::Implementation::loadModule( ModuleDecl *MD = nullptr; ASTContext &ctx = getNameImporter().getContext(); - // `CxxStdlib` is the only accepted spelling of the C++ stdlib module name. - if (path.front().Item.is("std") || - path.front().Item.str().starts_with("std_")) - return nullptr; if (path.front().Item == ctx.Id_CxxStdlib) { ImportPath::Builder adjustedPath(ctx.getIdentifier("std"), importLoc); adjustedPath.append(path.getSubmodulePath()); @@ -8891,7 +8887,7 @@ bool importer::isCxxStdModule(StringRef moduleName, bool IsSystem) { ImportPath::Builder ClangImporter::Implementation::getSwiftModulePath(const clang::Module *M) { ImportPath::Builder builder; while (M) { - if (!M->isSubModule() && isCxxStdModule(M)) + if (!M->isSubModule() && M->Name == "std") builder.push_back(SwiftContext.Id_CxxStdlib); else builder.push_back(SwiftContext.getIdentifier(M->Name)); diff --git a/lib/Sema/ImportResolution.cpp b/lib/Sema/ImportResolution.cpp index 3c4f4325f4674..f8de48dc4ef18 100644 --- a/lib/Sema/ImportResolution.cpp +++ b/lib/Sema/ImportResolution.cpp @@ -359,6 +359,26 @@ static bool isSubmodule(const ModuleDecl* M) { void ImportResolver::visitImportDecl(ImportDecl *ID) { assert(unboundImports.empty()); + // `CxxStdlib` is the only accepted spelling of the C++ stdlib module name. + ImportPath::Builder builder; + const ImportPath path = ID->getRealImportPath(builder); + const llvm::StringRef front = path.front().Item.str(); + if (front == "std" || front.starts_with("std_")) { + SmallString<64> modulePathStr; + path.getString(modulePathStr); + auto diagKind = ctx.LangOpts.DebuggerSupport ? diag::sema_no_import_repl + : diag::sema_no_import; + const SourceLoc importLoc = ID->getLoc(); + const ImportPath sourcePath = ID->getImportPath(); + const llvm::StringRef sourceFront = sourcePath.front().Item.str(); + ctx.Diags.diagnose(importLoc, diagKind, modulePathStr); + ctx.Diags.diagnose(importLoc, diag::did_you_mean_cxxstdlib) + .fixItReplaceChars(importLoc, importLoc.getAdvancedLoc(sourceFront.size()), "CxxStdlib"); + if (front != sourceFront) + ctx.Diags.diagnose(importLoc, diag::sema_module_aliased, sourceFront, front); + return; + } + unboundImports.emplace_back(ID); bindPendingImports(); } @@ -513,13 +533,15 @@ UnboundImport::getTopLevelModule(ModuleDecl *M, SourceFile &SF) { // MARK: Implicit imports //===----------------------------------------------------------------------===// -static void tryStdlibFixit(ASTContext &ctx, - StringRef moduleName, - SourceLoc loc) { - if (moduleName.starts_with("std")) { - ctx.Diags.diagnose(loc, diag::did_you_mean_cxxstdlib) - .fixItReplaceChars(loc, loc.getAdvancedLoc(3), "CxxStdlib"); +ImportPath::Module getRealModulePath(ImportPath::Builder &builder, ImportPath::Module path, ASTContext &ctx) { + for (size_t i = 0; i < path.size(); i++) { + if (i == 0) { + builder.push_back(ctx.getRealModuleName(path[i].Item)); + } else { + builder.push_back(path[i]); + } } + return builder.get().getModulePath(false); } static void diagnoseNoSuchModule(ModuleDecl *importingModule, @@ -534,13 +556,20 @@ static void diagnoseNoSuchModule(ModuleDecl *importingModule, importingModule->getName()); } else { SmallString<64> modulePathStr; - modulePath.getString(modulePathStr); + ImportPath::Builder builder; + ImportPath::Module realModulePath = getRealModulePath(builder, modulePath, ctx); + realModulePath.getString(modulePathStr); auto diagKind = diag::sema_no_import; if (nonfatalInREPL && ctx.LangOpts.DebuggerSupport) diagKind = diag::sema_no_import_repl; ctx.Diags.diagnose(importLoc, diagKind, modulePathStr); - tryStdlibFixit(ctx, modulePathStr, importLoc); + + const llvm::StringRef sourceFront = modulePath.front().Item.str(); + const llvm::StringRef realFront = realModulePath.front().Item.str(); + if (realFront != sourceFront) + ctx.Diags.diagnose(importLoc, diag::sema_module_aliased, sourceFront, + realFront); } if (ctx.SearchPathOpts.getSDKPath().empty() && diff --git a/test/ImportResolution/import-alias-fail.swift b/test/ImportResolution/import-alias-fail.swift new file mode 100644 index 0000000000000..8a033b0292c44 --- /dev/null +++ b/test/ImportResolution/import-alias-fail.swift @@ -0,0 +1,5 @@ +// RUN: %target-typecheck-verify-swift -module-alias Foo=Bar + +import Foo // expected-error{{no such module 'Bar'}} +// expected-note@-1{{module name 'Foo' is aliased to 'Bar'}} + diff --git a/test/ImportResolution/import-std-fail.swift b/test/ImportResolution/import-std-fail.swift new file mode 100644 index 0000000000000..96b5d44a556bc --- /dev/null +++ b/test/ImportResolution/import-std-fail.swift @@ -0,0 +1,42 @@ +// RUN: %empty-directory(%t) +// RUN: split-file %s %t + +// These errors are fatal, so test each one separately + +// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/a.swift +// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/b.swift +// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/c.swift +// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/d.swift +// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/e.swift -verify-additional-prefix aliased- -module-alias FooBar=std_foo_bar +// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/e.swift -verify-additional-prefix unaliased- +// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/f.swift -verify-additional-prefix aliased- -module-alias X=std_x_h +// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/f.swift -verify-additional-prefix unaliased- + +//--- a.swift +import std // expected-error{{no such module 'std'}} + // expected-note@-1{{did you mean 'CxxStdlib'?}}{{8-11=CxxStdlib}} {{none}} + +//--- b.swift +import std_ // expected-error{{no such module 'std_'}} + // expected-note@-1{{did you mean 'CxxStdlib'?}}{{8-12=CxxStdlib}} {{none}} + +//--- c.swift +import std_error_h // expected-error{{no such module 'std_error_h'}} + // expected-note@-1{{did you mean 'CxxStdlib'?}}{{8-19=CxxStdlib}} {{none}} + +//--- d.swift +import std_core.math.abs // expected-error{{no such module 'std_core.math.abs'}} + // expected-note@-1{{did you mean 'CxxStdlib'?}}{{8-16=CxxStdlib}} {{none}} + +//--- e.swift +import FooBar // expected-aliased-error{{no such module 'std_foo_bar'}} + // expected-aliased-note@-1{{did you mean 'CxxStdlib'?}}{{8-14=CxxStdlib}} {{none}} + // expected-aliased-note@-2{{module name 'FooBar' is aliased to 'std_foo_bar'}} {{none}} + // expected-unaliased-error@-3{{no such module 'FooBar'}} + +//--- f.swift +import X +// expected-aliased-error@-1{{no such module 'std_x_h'}} +// expected-aliased-note@-2{{did you mean 'CxxStdlib'?}}{{8-9=CxxStdlib}} {{none}} +// expected-aliased-note@-3{{module name 'X' is aliased to 'std_x_h'}} {{none}} +// expected-unaliased-error@-4{{no such module 'X'}} diff --git a/test/Interop/Cxx/swiftify-import/transitive-std-core-import.swift b/test/Interop/Cxx/swiftify-import/transitive-std-core-import.swift new file mode 100644 index 0000000000000..78b86f2393df3 --- /dev/null +++ b/test/Interop/Cxx/swiftify-import/transitive-std-core-import.swift @@ -0,0 +1,33 @@ +// REQUIRES: swift_feature_SafeInteropWrappers + +// RUN: %empty-directory(%t) +// RUN: split-file %s %t +// RUN: %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -o %t/Test.swiftmodule -I %t/Inputs -enable-experimental-feature SafeInteropWrappers -strict-memory-safety -warnings-as-errors -Xcc -Werror %t/test.swift -cxx-interoperability-mode=default -Xcc -std=c++20 + +// Test that implicit imports can refer to std_core etc., even though Swift source code may not. + +//--- Inputs/module.modulemap +module TestClang { + header "test.h" + requires cplusplus + export std_core.cstddef.size_t + export span +} + +//--- Inputs/test.h + +#include <__cstddef/size_t.h> +#include +#include + +using FloatSpan = std::span; + +size_t foo(FloatSpan x __noescape); + +//--- test.swift +import TestClang + +func test(x: Span) { + let _ = foo(x) +} + From a97da39e5d9dfeccc55dd7697375bd79f2fc8d3c Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 6 Oct 2025 18:02:26 -0700 Subject: [PATCH 2/2] only run transitive-std-core-import.swift on new libc++ --- .../Cxx/swiftify-import/transitive-std-core-import.swift | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/Interop/Cxx/swiftify-import/transitive-std-core-import.swift b/test/Interop/Cxx/swiftify-import/transitive-std-core-import.swift index 78b86f2393df3..c304506fd07db 100644 --- a/test/Interop/Cxx/swiftify-import/transitive-std-core-import.swift +++ b/test/Interop/Cxx/swiftify-import/transitive-std-core-import.swift @@ -1,8 +1,13 @@ // REQUIRES: swift_feature_SafeInteropWrappers +// REQUIRES: OS=macosx +// Don't run this test with libc++ versions 17-19, when the top-level std module was split into multiple top-level modules. // RUN: %empty-directory(%t) -// RUN: split-file %s %t -// RUN: %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -o %t/Test.swiftmodule -I %t/Inputs -enable-experimental-feature SafeInteropWrappers -strict-memory-safety -warnings-as-errors -Xcc -Werror %t/test.swift -cxx-interoperability-mode=default -Xcc -std=c++20 +// RUN: %target-clangxx %S/../stdlib/Inputs/check-libcxx-version.cpp -o %t/check-libcxx-version +// RUN: %target-codesign %t/check-libcxx-version + +// RUN: %target-run %t/check-libcxx-version || split-file %s %t +// RUN: %target-run %t/check-libcxx-version || %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -o %t/Test.swiftmodule -I %t/Inputs -enable-experimental-feature SafeInteropWrappers -strict-memory-safety -warnings-as-errors -Xcc -Werror %t/test.swift -cxx-interoperability-mode=default -Xcc -std=c++20 // Test that implicit imports can refer to std_core etc., even though Swift source code may not.