Skip to content

Commit 8d4e81c

Browse files
committed
[ImportResolution] Only disallow explicit std imports (allow implicit)
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
1 parent d653b0c commit 8d4e81c

File tree

6 files changed

+120
-13
lines changed

6 files changed

+120
-13
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,8 @@ WARNING(sema_import_current_module_with_file,none,
875875
(StringRef, Identifier))
876876
ERROR(sema_opening_import,Fatal,
877877
"opening import file for module %0: %1", (Identifier, StringRef))
878+
NOTE(sema_module_aliased,none,
879+
"module name '%0' is aliased to '%1'", (StringRef, StringRef))
878880

879881
REMARK(serialization_skipped_invalid_decl,none,
880882
"serialization skipped invalid %kind0",

lib/ClangImporter/ClangImporter.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2415,10 +2415,6 @@ ModuleDecl *ClangImporter::Implementation::loadModule(
24152415
ModuleDecl *MD = nullptr;
24162416
ASTContext &ctx = getNameImporter().getContext();
24172417

2418-
// `CxxStdlib` is the only accepted spelling of the C++ stdlib module name.
2419-
if (path.front().Item.is("std") ||
2420-
path.front().Item.str().starts_with("std_"))
2421-
return nullptr;
24222418
if (path.front().Item == ctx.Id_CxxStdlib) {
24232419
ImportPath::Builder adjustedPath(ctx.getIdentifier("std"), importLoc);
24242420
adjustedPath.append(path.getSubmodulePath());
@@ -8891,7 +8887,7 @@ bool importer::isCxxStdModule(StringRef moduleName, bool IsSystem) {
88918887
ImportPath::Builder ClangImporter::Implementation::getSwiftModulePath(const clang::Module *M) {
88928888
ImportPath::Builder builder;
88938889
while (M) {
8894-
if (!M->isSubModule() && isCxxStdModule(M))
8890+
if (!M->isSubModule() && M->Name == "std")
88958891
builder.push_back(SwiftContext.Id_CxxStdlib);
88968892
else
88978893
builder.push_back(SwiftContext.getIdentifier(M->Name));

lib/Sema/ImportResolution.cpp

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,26 @@ static bool isSubmodule(const ModuleDecl* M) {
359359
void ImportResolver::visitImportDecl(ImportDecl *ID) {
360360
assert(unboundImports.empty());
361361

362+
// `CxxStdlib` is the only accepted spelling of the C++ stdlib module name.
363+
ImportPath::Builder builder;
364+
const ImportPath path = ID->getRealImportPath(builder);
365+
const llvm::StringRef front = path.front().Item.str();
366+
if (front == "std" || front.starts_with("std_")) {
367+
SmallString<64> modulePathStr;
368+
path.getString(modulePathStr);
369+
auto diagKind = ctx.LangOpts.DebuggerSupport ? diag::sema_no_import_repl
370+
: diag::sema_no_import;
371+
const SourceLoc importLoc = ID->getLoc();
372+
const ImportPath sourcePath = ID->getImportPath();
373+
const llvm::StringRef sourceFront = sourcePath.front().Item.str();
374+
ctx.Diags.diagnose(importLoc, diagKind, modulePathStr);
375+
ctx.Diags.diagnose(importLoc, diag::did_you_mean_cxxstdlib)
376+
.fixItReplaceChars(importLoc, importLoc.getAdvancedLoc(sourceFront.size()), "CxxStdlib");
377+
if (front != sourceFront)
378+
ctx.Diags.diagnose(importLoc, diag::sema_module_aliased, sourceFront, front);
379+
return;
380+
}
381+
362382
unboundImports.emplace_back(ID);
363383
bindPendingImports();
364384
}
@@ -513,13 +533,15 @@ UnboundImport::getTopLevelModule(ModuleDecl *M, SourceFile &SF) {
513533
// MARK: Implicit imports
514534
//===----------------------------------------------------------------------===//
515535

516-
static void tryStdlibFixit(ASTContext &ctx,
517-
StringRef moduleName,
518-
SourceLoc loc) {
519-
if (moduleName.starts_with("std")) {
520-
ctx.Diags.diagnose(loc, diag::did_you_mean_cxxstdlib)
521-
.fixItReplaceChars(loc, loc.getAdvancedLoc(3), "CxxStdlib");
536+
ImportPath::Module getRealModulePath(ImportPath::Builder &builder, ImportPath::Module path, ASTContext &ctx) {
537+
for (size_t i = 0; i < path.size(); i++) {
538+
if (i == 0) {
539+
builder.push_back(ctx.getRealModuleName(path[i].Item));
540+
} else {
541+
builder.push_back(path[i]);
542+
}
522543
}
544+
return builder.get().getModulePath(false);
523545
}
524546

525547
static void diagnoseNoSuchModule(ModuleDecl *importingModule,
@@ -534,13 +556,20 @@ static void diagnoseNoSuchModule(ModuleDecl *importingModule,
534556
importingModule->getName());
535557
} else {
536558
SmallString<64> modulePathStr;
537-
modulePath.getString(modulePathStr);
559+
ImportPath::Builder builder;
560+
ImportPath::Module realModulePath = getRealModulePath(builder, modulePath, ctx);
561+
realModulePath.getString(modulePathStr);
538562

539563
auto diagKind = diag::sema_no_import;
540564
if (nonfatalInREPL && ctx.LangOpts.DebuggerSupport)
541565
diagKind = diag::sema_no_import_repl;
542566
ctx.Diags.diagnose(importLoc, diagKind, modulePathStr);
543-
tryStdlibFixit(ctx, modulePathStr, importLoc);
567+
568+
const llvm::StringRef sourceFront = modulePath.front().Item.str();
569+
const llvm::StringRef realFront = realModulePath.front().Item.str();
570+
if (realFront != sourceFront)
571+
ctx.Diags.diagnose(importLoc, diag::sema_module_aliased, sourceFront,
572+
realFront);
544573
}
545574

546575
if (ctx.SearchPathOpts.getSDKPath().empty() &&
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// RUN: %target-typecheck-verify-swift -module-alias Foo=Bar
2+
3+
import Foo // expected-error{{no such module 'Bar'}}
4+
// expected-note@-1{{module name 'Foo' is aliased to 'Bar'}}
5+
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
// These errors are fatal, so test each one separately
5+
6+
// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/a.swift
7+
// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/b.swift
8+
// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/c.swift
9+
// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/d.swift
10+
// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/e.swift -verify-additional-prefix aliased- -module-alias FooBar=std_foo_bar
11+
// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/e.swift -verify-additional-prefix unaliased-
12+
// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/f.swift -verify-additional-prefix aliased- -module-alias X=std_x_h
13+
// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default %t/f.swift -verify-additional-prefix unaliased-
14+
15+
//--- a.swift
16+
import std // expected-error{{no such module 'std'}}
17+
// expected-note@-1{{did you mean 'CxxStdlib'?}}{{8-11=CxxStdlib}} {{none}}
18+
19+
//--- b.swift
20+
import std_ // expected-error{{no such module 'std_'}}
21+
// expected-note@-1{{did you mean 'CxxStdlib'?}}{{8-12=CxxStdlib}} {{none}}
22+
23+
//--- c.swift
24+
import std_error_h // expected-error{{no such module 'std_error_h'}}
25+
// expected-note@-1{{did you mean 'CxxStdlib'?}}{{8-19=CxxStdlib}} {{none}}
26+
27+
//--- d.swift
28+
import std_core.math.abs // expected-error{{no such module 'std_core.math.abs'}}
29+
// expected-note@-1{{did you mean 'CxxStdlib'?}}{{8-16=CxxStdlib}} {{none}}
30+
31+
//--- e.swift
32+
import FooBar // expected-aliased-error{{no such module 'std_foo_bar'}}
33+
// expected-aliased-note@-1{{did you mean 'CxxStdlib'?}}{{8-14=CxxStdlib}} {{none}}
34+
// expected-aliased-note@-2{{module name 'FooBar' is aliased to 'std_foo_bar'}} {{none}}
35+
// expected-unaliased-error@-3{{no such module 'FooBar'}}
36+
37+
//--- f.swift
38+
import X
39+
// expected-aliased-error@-1{{no such module 'std_x_h'}}
40+
// expected-aliased-note@-2{{did you mean 'CxxStdlib'?}}{{8-9=CxxStdlib}} {{none}}
41+
// expected-aliased-note@-3{{module name 'X' is aliased to 'std_x_h'}} {{none}}
42+
// expected-unaliased-error@-4{{no such module 'X'}}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// REQUIRES: swift_feature_SafeInteropWrappers
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: split-file %s %t
5+
// 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
6+
7+
// Test that implicit imports can refer to std_core etc., even though Swift source code may not.
8+
9+
//--- Inputs/module.modulemap
10+
module TestClang {
11+
header "test.h"
12+
requires cplusplus
13+
export std_core.cstddef.size_t
14+
export span
15+
}
16+
17+
//--- Inputs/test.h
18+
19+
#include <__cstddef/size_t.h>
20+
#include <span>
21+
#include <lifetimebound.h>
22+
23+
using FloatSpan = std::span<const float>;
24+
25+
size_t foo(FloatSpan x __noescape);
26+
27+
//--- test.swift
28+
import TestClang
29+
30+
func test(x: Span<Float>) {
31+
let _ = foo(x)
32+
}
33+

0 commit comments

Comments
 (0)