Skip to content

Commit b90f789

Browse files
zoecarverzoecarver
authored
Merge pull request #64438 from zoecarver/fixits-for-clang-types-swiftpm-2
2 parents b11b897 + 1513df6 commit b90f789

11 files changed

+137
-30
lines changed

‎include/swift/AST/ClangModuleLoader.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class Preprocessor;
2727
class Sema;
2828
class TargetInfo;
2929
class Type;
30+
class SourceLocation;
3031
} // namespace clang
3132

3233
namespace swift {
@@ -294,6 +295,8 @@ class ClangModuleLoader : public ModuleLoader {
294295

295296
virtual const clang::TypedefType *
296297
getTypeDefForCXXCFOptionsDefinition(const clang::Decl *candidateDecl) = 0;
298+
299+
virtual SourceLoc importSourceLocation(clang::SourceLocation loc) = 0;
297300
};
298301

299302
/// Describes a C++ template instantiation error.

‎include/swift/AST/DiagnosticsClangImporter.def

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,9 @@ NOTE(reference_passed_by_value, none, "function uses foreign reference type "
169169
"C++ Interop User Manual).",
170170
(StringRef, StringRef))
171171
NOTE(record_not_automatically_importable, none, "record '%0' is not "
172-
"automatically importable: %1. "
173-
"Refer to the C++ Interop User "
174-
"Manual to classify this type.",
172+
"automatically available: %1. "
173+
"Does this type have reference "
174+
"semantics?",
175175
(StringRef, StringRef))
176176

177177
NOTE(projection_value_not_imported, none, "C++ method '%0' that returns a value "
@@ -186,10 +186,10 @@ NOTE(projection_reference_not_imported, none, "C++ method '%0' that returns a re
186186
NOTE(projection_may_return_interior_ptr, none, "C++ method '%0' may return an "
187187
"interior pointer. ",
188188
(StringRef))
189-
NOTE(mark_self_contained, none, "Mark type '%0' as 'self_contained' in C++ to "
189+
NOTE(mark_self_contained, none, "Mark type '%0' as 'SELF_CONTAINED' in C++ to "
190190
"make methods that use it available in Swift. ",
191191
(StringRef))
192-
NOTE(mark_safe_to_import, none, "Mark method '%0' as 'safe_to_import' in C++ to "
192+
NOTE(mark_safe_to_import, none, "Mark method '%0' as 'SAFE_TO_IMPORT' in C++ to "
193193
"make it available in Swift. ",
194194
(StringRef))
195195

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,8 @@ ERROR(expect_compile_time_const,none,
764764

765765
ERROR(sema_no_import,Fatal,
766766
"no such module '%0'", (StringRef))
767+
NOTE(did_you_mean_cxxstdlib,none,
768+
"did you mean 'CxxStdlib'?'", ())
767769
ERROR(sema_no_import_target,Fatal,
768770
"could not find module '%0' for target '%1'; "
769771
"found: %2, at: %3", (StringRef, StringRef, StringRef, StringRef))

‎include/swift/ClangImporter/ClangImporter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,8 @@ class ClangImporter final : public ClangModuleLoader {
565565

566566
const clang::TypedefType *getTypeDefForCXXCFOptionsDefinition(
567567
const clang::Decl *candidateDecl) override;
568+
569+
SourceLoc importSourceLocation(clang::SourceLocation loc) override;
568570
};
569571

570572
ImportDecl *createImportDecl(ASTContext &Ctx, DeclContext *DC, ClangNode ClangN,

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4402,12 +4402,24 @@ ClangImporter::Implementation::importDiagnosticTargetFromLookupTableEntry(
44024402
"MacroInfo or ModuleMacro pointer");
44034403
}
44044404

4405+
static void diagnoseForeignReferenceTypeFixit(ClangImporter::Implementation &Impl,
4406+
HeaderLoc loc, Diagnostic diag) {
4407+
auto importedLoc =
4408+
Impl.SwiftContext.getClangModuleLoader()->importSourceLocation(loc.clangLoc);
4409+
Impl.diagnose(loc, diag)
4410+
.fixItInsert(importedLoc, "SWIFT_REFERENCE_TYPE(<#retain#>, <#release#>) ");
4411+
}
4412+
44054413
bool ClangImporter::Implementation::emitDiagnosticsForTarget(
44064414
ImportDiagnosticTarget target, clang::SourceLocation fallbackLoc) {
44074415
for (auto it = ImportDiagnostics[target].rbegin();
44084416
it != ImportDiagnostics[target].rend(); ++it) {
44094417
HeaderLoc loc = HeaderLoc(it->loc.isValid() ? it->loc : fallbackLoc);
4410-
diagnose(loc, it->diag);
4418+
if (it->diag.getID() == diag::record_not_automatically_importable.ID) {
4419+
diagnoseForeignReferenceTypeFixit(*this, loc, it->diag);
4420+
} else {
4421+
diagnose(loc, it->diag);
4422+
}
44114423
}
44124424
return ImportDiagnostics[target].size();
44134425
}
@@ -6316,6 +6328,12 @@ void ClangImporter::diagnoseMemberValue(const DeclName &name,
63166328
}
63176329
}
63186330

6331+
SourceLoc ClangImporter::importSourceLocation(clang::SourceLocation loc) {
6332+
auto &bufferImporter = Impl.getBufferImporterForDiagnostics();
6333+
return bufferImporter.resolveSourceLocation(
6334+
getClangASTContext().getSourceManager(), loc);
6335+
}
6336+
63196337
static bool hasImportAsRefAttr(const clang::RecordDecl *decl) {
63206338
return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
63216339
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))

‎lib/ClangImporter/ImporterImpl.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -917,10 +917,10 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
917917
/// interface. Use this to diagnose issues with declarations that are not
918918
/// imported or that are not reflected in a generated interface.
919919
template<typename ...Args>
920-
void diagnose(HeaderLoc loc, Args &&...args) {
920+
InFlightDiagnostic diagnose(HeaderLoc loc, Args &&...args) {
921921
// If we're in the middle of pretty-printing, suppress diagnostics.
922922
if (SwiftContext.Diags.isPrettyPrintingDecl()) {
923-
return;
923+
return InFlightDiagnostic();
924924
}
925925

926926
auto swiftLoc = loc.fallbackLoc;
@@ -932,7 +932,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
932932
loc.clangLoc);
933933
}
934934

935-
SwiftContext.Diags.diagnose(swiftLoc, std::forward<Args>(args)...);
935+
return SwiftContext.Diags.diagnose(swiftLoc, std::forward<Args>(args)...);
936936
}
937937

938938
void addImportDiagnostic(

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3968,6 +3968,10 @@ void MissingMemberFailure::diagnoseUnsafeCxxMethod(SourceLoc loc,
39683968
->getName()
39693969
.str();
39703970

3971+
auto methodClangLoc = cxxMethod->getLocation();
3972+
auto methodSwiftLoc =
3973+
ctx.getClangModuleLoader()->importSourceLocation(methodClangLoc);
3974+
39713975
// Rewrite front() and back() as first and last.
39723976
if ((name.getBaseIdentifier().is("front") ||
39733977
name.getBaseIdentifier().is("back")) &&
@@ -4013,7 +4017,9 @@ void MissingMemberFailure::diagnoseUnsafeCxxMethod(SourceLoc loc,
40134017
ctx.Diags.diagnose(loc, diag::iterator_method_unavailable,
40144018
name.getBaseIdentifier().str());
40154019
ctx.Diags.diagnose(loc, diag::replace_with_nil)
4016-
.fixItReplaceChars(loc, callExpr->getArgs()->getEndLoc(), "nil");
4020+
.fixItReplaceChars(
4021+
getAnchor().getStartLoc(),
4022+
callExpr->getArgs()->getEndLoc().getAdvancedLoc(1), "nil");
40174023
} else {
40184024
ctx.Diags.diagnose(loc, diag::iterator_method_unavailable,
40194025
name.getBaseIdentifier().str());
@@ -4024,8 +4030,11 @@ void MissingMemberFailure::diagnoseUnsafeCxxMethod(SourceLoc loc,
40244030
name.getBaseIdentifier().str(), returnTypeStr);
40254031
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
40264032
name.getBaseIdentifier().str());
4027-
ctx.Diags.diagnose(loc, diag::mark_safe_to_import,
4028-
name.getBaseIdentifier().str());
4033+
ctx.Diags
4034+
.diagnose(methodSwiftLoc, diag::mark_safe_to_import,
4035+
name.getBaseIdentifier().str())
4036+
.fixItInsert(methodSwiftLoc,
4037+
" SAFE_TO_IMPORT ");
40294038
} else if (cxxMethod->getReturnType()->isReferenceType()) {
40304039
// Rewrite a call to .at(42) as a subscript.
40314040
if (name.getBaseIdentifier().is("at") &&
@@ -4034,11 +4043,7 @@ void MissingMemberFailure::diagnoseUnsafeCxxMethod(SourceLoc loc,
40344043
auto dotExpr = getAsExpr<UnresolvedDotExpr>(anchor);
40354044
auto callExpr = getAsExpr<CallExpr>(findParentExpr(dotExpr));
40364045

4037-
ctx.Diags.diagnose(loc, diag::projection_reference_not_imported,
4038-
name.getBaseIdentifier().str(), returnTypeStr);
4039-
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
4040-
name.getBaseIdentifier().str());
4041-
ctx.Diags.diagnose(loc, diag::at_to_subscript)
4046+
ctx.Diags.diagnose(dotExpr->getDotLoc(), diag::at_to_subscript)
40424047
.fixItRemove(
40434048
{dotExpr->getDotLoc(), callExpr->getArgs()->getStartLoc()})
40444049
.fixItReplaceChars(
@@ -4047,13 +4052,20 @@ void MissingMemberFailure::diagnoseUnsafeCxxMethod(SourceLoc loc,
40474052
.fixItReplaceChars(
40484053
callExpr->getArgs()->getEndLoc(),
40494054
callExpr->getArgs()->getEndLoc().getAdvancedLoc(1), "]");
4050-
} else {
40514055
ctx.Diags.diagnose(loc, diag::projection_reference_not_imported,
40524056
name.getBaseIdentifier().str(), returnTypeStr);
40534057
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
40544058
name.getBaseIdentifier().str());
4055-
ctx.Diags.diagnose(loc, diag::mark_safe_to_import,
4059+
} else {
4060+
ctx.Diags.diagnose(loc, diag::projection_reference_not_imported,
4061+
name.getBaseIdentifier().str(), returnTypeStr);
4062+
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
40564063
name.getBaseIdentifier().str());
4064+
ctx.Diags
4065+
.diagnose(methodSwiftLoc, diag::mark_safe_to_import,
4066+
name.getBaseIdentifier().str())
4067+
.fixItInsert(methodSwiftLoc,
4068+
" SAFE_TO_IMPORT ");
40574069
}
40584070
} else if (cxxMethod->getReturnType()->isRecordType()) {
40594071
if (auto cxxRecord = dyn_cast<clang::CXXRecordDecl>(
@@ -4067,13 +4079,23 @@ void MissingMemberFailure::diagnoseUnsafeCxxMethod(SourceLoc loc,
40674079
} else {
40684080
assert(methodSemantics ==
40694081
CxxRecordSemanticsKind::UnsafePointerMember);
4082+
4083+
auto baseSwiftLoc = ctx.getClangModuleLoader()->importSourceLocation(
4084+
cxxRecord->getLocation());
4085+
40704086
ctx.Diags.diagnose(loc, diag::projection_value_not_imported,
40714087
name.getBaseIdentifier().str(), returnTypeStr);
40724088
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
40734089
name.getBaseIdentifier().str());
4074-
ctx.Diags.diagnose(loc, diag::mark_safe_to_import,
4075-
name.getBaseIdentifier().str());
4076-
ctx.Diags.diagnose(loc, diag::mark_self_contained, returnTypeStr);
4090+
ctx.Diags
4091+
.diagnose(methodSwiftLoc, diag::mark_safe_to_import,
4092+
name.getBaseIdentifier().str())
4093+
.fixItInsert(methodSwiftLoc,
4094+
" SAFE_TO_IMPORT ");
4095+
ctx.Diags
4096+
.diagnose(baseSwiftLoc, diag::mark_self_contained, returnTypeStr)
4097+
.fixItInsert(baseSwiftLoc,
4098+
"SELF_CONTAINED ");
40774099
}
40784100
}
40794101
}

‎lib/Sema/ImportResolution.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,15 @@ UnboundImport::getTopLevelModule(ModuleDecl *M, SourceFile &SF) {
420420
// MARK: Implicit imports
421421
//===----------------------------------------------------------------------===//
422422

423+
static void tryStdlibFixit(ASTContext &ctx,
424+
StringRef moduleName,
425+
SourceLoc loc) {
426+
if (moduleName.startswith("std")) {
427+
ctx.Diags.diagnose(loc, diag::did_you_mean_cxxstdlib)
428+
.fixItReplaceChars(loc, loc.getAdvancedLoc(3), "CxxStdlib");
429+
}
430+
}
431+
423432
static void diagnoseNoSuchModule(ModuleDecl *importingModule,
424433
SourceLoc importLoc,
425434
ImportPath::Module modulePath,
@@ -438,6 +447,7 @@ static void diagnoseNoSuchModule(ModuleDecl *importingModule,
438447
if (nonfatalInREPL && ctx.LangOpts.DebuggerSupport)
439448
diagKind = diag::sema_no_import_repl;
440449
ctx.Diags.diagnose(importLoc, diagKind, modulePathStr);
450+
tryStdlibFixit(ctx, modulePathStr, importLoc);
441451
}
442452

443453
if (ctx.SearchPathOpts.getSDKPath().empty() &&

‎test/Interop/Cxx/class/fixit-add-safe-to-import-self-contained.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
// RUN: not %target-swift-frontend -typecheck -I %t/Inputs %t/test.swift -enable-experimental-cxx-interop 2>&1 | %FileCheck %s
4+
5+
//--- Inputs/module.modulemap
6+
module Test {
7+
header "test.h"
8+
requires cplusplus
9+
}
10+
11+
//--- Inputs/test.h
12+
struct Ptr { int *p; };
13+
14+
struct X {
15+
int *test() { }
16+
Ptr other() { }
17+
};
18+
19+
//--- test.swift
20+
21+
import Test
22+
23+
public func test(x: X) {
24+
// CHECK: note: Mark method 'test' as 'SAFE_TO_IMPORT' in C++ to make it available in Swift.
25+
// CHECK: int *test() { }
26+
// CHECK: ^
27+
// CHECK: SAFE_TO_IMPORT
28+
29+
x.test()
30+
31+
// CHECK: note: Mark method 'other' as 'SAFE_TO_IMPORT' in C++ to make it available in Swift.
32+
// CHECK: Ptr other() { }
33+
// CHECK: ^
34+
// CHECK: SAFE_TO_IMPORT
35+
36+
// CHECK: note: Mark type 'Ptr' as 'SELF_CONTAINED' in C++ to make methods that use it available in Swift.
37+
// CHECK: struct Ptr {
38+
// CHECK: ^
39+
// CHECK: SELF_CONTAINED
40+
x.other()
41+
}

‎test/Interop/Cxx/class/invalid-class-errors.swift

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,18 @@ struct Nested {
2727

2828
import Test
2929

30-
// CHECK: note: record 'A' is not automatically importable: does not have a copy constructor or destructor. Refer to the C++ Interop User Manual to classify this type.
30+
// CHECK: note: record 'A' is not automatically available: does not have a copy constructor or destructor. Does this type have reference semantics?
31+
// CHECK: struct A {
32+
// CHECK: ^
33+
// CHECK: SWIFT_REFERENCE_TYPE(<#retain#>, <#release#>)
3134
public func test(x: A) { }
32-
// CHECK: note: record 'B' is not automatically importable: does not have a copy constructor or destructor. Refer to the C++ Interop User Manual to classify this type.
35+
// CHECK: note: record 'B' is not automatically available: does not have a copy constructor or destructor. Does this type have reference semantics?
36+
// CHECK: struct {{.*}}B {
37+
// CHECK: ^
38+
// CHECK: SWIFT_REFERENCE_TYPE(<#retain#>, <#release#>)
3339
public func test(x: B) { }
34-
// CHECK: note: record 'Nested' is not automatically importable: does not have a copy constructor or destructor. Refer to the C++ Interop User Manual to classify this type.
35-
public func test(x: Namespace.Nested) { }
40+
// CHECK: note: record 'Nested' is not automatically available: does not have a copy constructor or destructor. Does this type have reference semantics?
41+
// CHECK: struct Nested {
42+
// CHECK: ^
43+
// CHECK: SWIFT_REFERENCE_TYPE(<#retain#>, <#release#>)
44+
public func test(x: Namespace.Nested) { }

0 commit comments

Comments
 (0)