Skip to content

Commit 9d1d031

Browse files
zoecarverzoecarver
committed
[nfc][cxx-interop] Add diagnostics when something cannot be imported.
1 parent 6acffbb commit 9d1d031

File tree

11 files changed

+281
-55
lines changed

11 files changed

+281
-55
lines changed

‎include/swift/AST/DiagnosticsClangImporter.def

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ WARNING(too_many_class_template_instantiations, none,
128128
"template instantiation for '%0' not imported: too many instantiations",
129129
(StringRef))
130130

131+
WARNING(api_pattern_attr_ignored, none,
132+
"'%0' swift attribute ignored on type '%1': type is not copyable or destructible",
133+
(StringRef, StringRef))
134+
131135
NOTE(macro_not_imported_unsupported_operator, none, "operator not supported in macro arithmetic", ())
132136
NOTE(macro_not_imported_unsupported_named_operator, none, "operator '%0' not supported in macro arithmetic", (StringRef))
133137
NOTE(macro_not_imported_invalid_string_literal, none, "invalid string literal", ())
@@ -140,18 +144,40 @@ NOTE(macro_not_imported_unsupported_structure, none, "macro '%0' not imported: s
140144
NOTE(macro_not_imported, none, "macro '%0' not imported", (StringRef))
141145

142146
NOTE(return_type_not_imported, none, "return type not imported", ())
143-
NOTE(parameter_type_not_imported, none, "parameter %0 not imported", (const clang::NamedDecl*))
144-
NOTE(incomplete_interface, none, "interface %0 is incomplete", (const clang::NamedDecl*))
145-
NOTE(incomplete_protocol, none, "protocol %0 is incomplete", (const clang::NamedDecl*))
146-
NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
147+
NOTE(parameter_type_not_imported, none, "parameter '%0' not imported", (const clang::NamedDecl*))
148+
NOTE(incomplete_interface, none, "interface '%0' is incomplete", (const clang::NamedDecl*))
149+
NOTE(incomplete_protocol, none, "protocol '%0' is incomplete", (const clang::NamedDecl*))
150+
NOTE(incomplete_record, none, "record '%0' is not defined (incomplete)", (StringRef))
151+
NOTE(record_over_aligned, none, "record '%0' is over aligned", (StringRef))
152+
NOTE(record_non_trivial_copy_destroy, none, "record '%0' is not trivial to copy/destroy", (StringRef))
153+
NOTE(record_is_dependent, none, "record '%0' is dependent", (StringRef))
154+
NOTE(record_parent_unimportable, none, "record %0's parent is not importable", (StringRef))
155+
NOTE(reference_passed_by_value, none, "function uses foreign reference type "
156+
"'%0' as a value in %1 types which breaks "
157+
"'import_reference' contract (outlined in "
158+
"C++ Interop User Manual).",
159+
(StringRef, StringRef))
160+
NOTE(record_not_automatically_importable, none, "record '%0' is not "
161+
"automatically importable: %1. "
162+
"Refer to the C++ Interop User "
163+
"Manual to classify this type.",
164+
(StringRef, StringRef))
165+
NOTE(projection_not_imported, none, "C++ method '%0' that returns unsafe "
166+
"projection of type '%1' not imported",
167+
(StringRef, StringRef))
168+
NOTE(dont_use_iterator_api, none, "C++ method '%0' that returns an unsafe "
169+
"iterator not imported: use Swift Sequence "
170+
"APIs instead",
171+
(StringRef))
147172

148-
NOTE(record_field_not_imported, none, "field %0 not imported", (const clang::NamedDecl*))
149-
NOTE(invoked_func_not_imported, none, "function %0 not imported", (const clang::NamedDecl*))
150-
NOTE(record_method_not_imported, none, "method %0 not imported", (const clang::NamedDecl*))
151-
NOTE(objc_property_not_imported, none, "property %0 not imported", (const clang::NamedDecl*))
173+
NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
174+
NOTE(record_field_not_imported, none, "field '%0' not imported", (const clang::NamedDecl*))
175+
NOTE(invoked_func_not_imported, none, "function '%0' not imported", (const clang::NamedDecl*))
176+
NOTE(record_method_not_imported, none, "method '%0' not imported", (const clang::NamedDecl*))
177+
NOTE(objc_property_not_imported, none, "property '%0' not imported", (const clang::NamedDecl*))
152178

153-
NOTE(forward_declared_interface_label, none, "interface %0 forward declared here", (const clang::NamedDecl*))
154-
NOTE(forward_declared_protocol_label, none, "protocol %0 forward declared here", (const clang::NamedDecl*))
179+
NOTE(forward_declared_interface_label, none, "interface '%0' forward declared here", (const clang::NamedDecl*))
180+
NOTE(forward_declared_protocol_label, none, "protocol '%0' forward declared here", (const clang::NamedDecl*))
155181

156182
#define UNDEFINE_DIAGNOSTIC_MACROS
157183
#include "DefineDiagnosticMacros.h"

‎include/swift/ClangImporter/ClangImporterRequests.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,8 @@ enum class CxxRecordSemanticsKind {
186186
// An API that has be annotated as explicitly unsafe, but still importable.
187187
// TODO: we should rename these APIs.
188188
ExplicitlyUnsafe,
189+
// A record that is either not copyable or not destructible.
190+
MissingLifetimeOperation,
189191
// A record that has custom lifetime operations which we cannot prove are
190192
// safe.
191193
UnsafeLifetimeOperation,
@@ -195,8 +197,11 @@ enum class CxxRecordSemanticsKind {
195197

196198
struct CxxRecordSemanticsDescriptor final {
197199
const clang::CXXRecordDecl *decl;
200+
ASTContext &ctx;
198201

199-
CxxRecordSemanticsDescriptor(const clang::CXXRecordDecl *decl) : decl(decl) {}
202+
CxxRecordSemanticsDescriptor(const clang::CXXRecordDecl *decl,
203+
ASTContext &ctx)
204+
: decl(decl), ctx(ctx) {}
200205

201206
friend llvm::hash_code hash_value(const CxxRecordSemanticsDescriptor &desc) {
202207
return llvm::hash_combine(desc.decl);
@@ -241,8 +246,11 @@ class CxxRecordSemantics
241246

242247
struct SafeUseOfCxxDeclDescriptor final {
243248
const clang::Decl *decl;
249+
ASTContext &ctx;
244250

245-
SafeUseOfCxxDeclDescriptor(const clang::Decl *decl) : decl(decl) {}
251+
SafeUseOfCxxDeclDescriptor(const clang::Decl *decl,
252+
ASTContext &ctx)
253+
: decl(decl), ctx(ctx) {}
246254

247255
friend llvm::hash_code hash_value(const SafeUseOfCxxDeclDescriptor &desc) {
248256
return llvm::hash_combine(desc.decl);

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5939,9 +5939,21 @@ CxxRecordSemantics::evaluate(Evaluator &evaluator,
59395939
return CxxRecordSemanticsKind::Reference;
59405940
}
59415941

5942-
// TODO: diagnose if the decl also has any attrs.
59435942
if (!hasRequiredValueTypeOperations(decl)) {
5944-
return CxxRecordSemanticsKind::UnsafeLifetimeOperation;
5943+
if (hasUnsafeAPIAttr(decl))
5944+
desc.ctx.Diags.diagnose({}, diag::api_pattern_attr_ignored,
5945+
"import_unsafe",
5946+
decl->getNameAsString());
5947+
if (hasOwnedValueAttr(decl))
5948+
desc.ctx.Diags.diagnose({}, diag::api_pattern_attr_ignored,
5949+
"import_owned",
5950+
decl->getNameAsString());
5951+
if (hasIteratorAPIAttr(decl))
5952+
desc.ctx.Diags.diagnose({}, diag::api_pattern_attr_ignored,
5953+
"import_iterator",
5954+
decl->getNameAsString());
5955+
5956+
return CxxRecordSemanticsKind::MissingLifetimeOperation;
59455957
}
59465958

59475959
if (hasUnsafeAPIAttr(decl)) {
@@ -5992,7 +6004,8 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
59926004
if (auto cxxRecordReturnType =
59936005
dyn_cast<clang::CXXRecordDecl>(returnType->getDecl())) {
59946006
auto semanticsKind = evaluateOrDefault(
5995-
evaluator, CxxRecordSemantics({cxxRecordReturnType}), {});
6007+
evaluator,
6008+
CxxRecordSemantics({cxxRecordReturnType, desc.ctx}), {});
59966009

59976010
if (semanticsKind == CxxRecordSemanticsKind::UnsafePointerMember ||
59986011
// Pretend all methods that return iterators are unsafe so protocol
@@ -6011,10 +6024,12 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
60116024
}
60126025

60136026
auto semanticsKind =
6014-
evaluateOrDefault(evaluator, CxxRecordSemantics({recordDecl}), {});
6027+
evaluateOrDefault(evaluator,
6028+
CxxRecordSemantics({recordDecl, desc.ctx}), {});
60156029

60166030
// Always unsafe.
6017-
if (semanticsKind == CxxRecordSemanticsKind::UnsafeLifetimeOperation)
6031+
if (semanticsKind == CxxRecordSemanticsKind::MissingLifetimeOperation ||
6032+
semanticsKind == CxxRecordSemanticsKind::UnsafeLifetimeOperation)
60186033
return false;
60196034

60206035
// Always OK.

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,7 +1950,8 @@ namespace {
19501950
bool recordHasReferenceSemantics(const clang::RecordDecl *decl) {
19511951
if (auto cxxRecord = dyn_cast<clang::CXXRecordDecl>(decl)) {
19521952
auto semanticsKind = evaluateOrDefault(
1953-
Impl.SwiftContext.evaluator, CxxRecordSemantics({cxxRecord}), {});
1953+
Impl.SwiftContext.evaluator,
1954+
CxxRecordSemantics({cxxRecord, Impl.SwiftContext}), {});
19541955
return semanticsKind == CxxRecordSemanticsKind::Reference;
19551956
}
19561957

@@ -1982,6 +1983,13 @@ namespace {
19821983
if (decl->isInterface())
19831984
return nullptr;
19841985

1986+
if (!decl->getDefinition()) {
1987+
Impl.addImportDiagnostic(
1988+
decl,
1989+
Diagnostic(diag::incomplete_record,
1990+
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())));
1991+
}
1992+
19851993
// FIXME: Figure out how to deal with incomplete types, since that
19861994
// notion doesn't exist in Swift.
19871995
decl = decl->getDefinition();
@@ -1991,12 +1999,22 @@ namespace {
19911999
}
19922000

19932001
// TODO(SR-13809): fix this once we support dependent types.
1994-
if (decl->getTypeForDecl()->isDependentType())
2002+
if (decl->getTypeForDecl()->isDependentType()) {
2003+
Impl.addImportDiagnostic(
2004+
decl,
2005+
Diagnostic(diag::record_is_dependent,
2006+
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())));
19952007
return nullptr;
2008+
}
19962009

19972010
// Don't import nominal types that are over-aligned.
1998-
if (Impl.isOverAligned(decl))
2011+
if (Impl.isOverAligned(decl)) {
2012+
Impl.addImportDiagnostic(
2013+
decl,
2014+
Diagnostic(diag::record_over_aligned,
2015+
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())));
19992016
return nullptr;
2017+
}
20002018

20012019
// FIXME: We should actually support strong ARC references and similar in
20022020
// C structs. That'll require some SIL and IRGen work, though.
@@ -2012,6 +2030,10 @@ namespace {
20122030
// imported function and the developer would be able to use it without
20132031
// referencing the name, which would sidestep our availability
20142032
// diagnostics.
2033+
Impl.addImportDiagnostic(
2034+
decl,
2035+
Diagnostic(diag::record_non_trivial_copy_destroy,
2036+
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())));
20152037
return nullptr;
20162038
}
20172039

@@ -2030,8 +2052,13 @@ namespace {
20302052

20312053
auto dc =
20322054
Impl.importDeclContextOf(decl, importedName.getEffectiveContext());
2033-
if (!dc)
2055+
if (!dc) {
2056+
Impl.addImportDiagnostic(
2057+
decl,
2058+
Diagnostic(diag::record_parent_unimportable,
2059+
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())));
20342060
return nullptr;
2061+
}
20352062

20362063
// Create the struct declaration and record it.
20372064
auto name = importedName.getDeclName().getBaseIdentifier();
@@ -2403,6 +2430,11 @@ namespace {
24032430
if (!Impl.SwiftContext.LangOpts.EnableCXXInterop)
24042431
return VisitRecordDecl(decl);
24052432

2433+
if (!decl->getDefinition()) {
2434+
Impl.addImportDiagnostic(decl, Diagnostic(diag::incomplete_record,
2435+
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())));
2436+
}
2437+
24062438
decl = decl->getDefinition();
24072439
if (!decl) {
24082440
forwardDeclaration = true;
@@ -2453,10 +2485,24 @@ namespace {
24532485

24542486
// It is import that we bail on an unimportable record *before* we import
24552487
// any of its members or cache the decl.
2456-
auto semanticsKind = evaluateOrDefault(Impl.SwiftContext.evaluator,
2457-
CxxRecordSemantics({decl}), {});
2458-
if (semanticsKind == CxxRecordSemanticsKind::UnsafeLifetimeOperation)
2488+
auto semanticsKind = evaluateOrDefault(
2489+
Impl.SwiftContext.evaluator,
2490+
CxxRecordSemantics({decl, Impl.SwiftContext}), {});
2491+
if (semanticsKind == CxxRecordSemanticsKind::MissingLifetimeOperation) {
2492+
Impl.addImportDiagnostic(
2493+
decl,
2494+
Diagnostic(diag::record_not_automatically_importable,
2495+
Impl.SwiftContext.AllocateCopy(decl->getNameAsString()),
2496+
"does not have a copy constructor or destructor"));
2497+
return nullptr;
2498+
} else if (semanticsKind == CxxRecordSemanticsKind::UnsafeLifetimeOperation) {
2499+
Impl.addImportDiagnostic(
2500+
decl,
2501+
Diagnostic(diag::record_not_automatically_importable,
2502+
Impl.SwiftContext.AllocateCopy(decl->getNameAsString()),
2503+
"has custom, potentially unsafe copy constructor or destructor"));
24592504
return nullptr;
2505+
}
24602506

24612507
return VisitRecordDecl(decl);
24622508
}
@@ -2634,8 +2680,10 @@ namespace {
26342680
if (auto parent = dyn_cast<clang::CXXRecordDecl>(
26352681
decl->getAnonField()->getParent())) {
26362682
auto semanticsKind = evaluateOrDefault(
2637-
Impl.SwiftContext.evaluator, CxxRecordSemantics({parent}), {});
2638-
if (semanticsKind == CxxRecordSemanticsKind::UnsafeLifetimeOperation)
2683+
Impl.SwiftContext.evaluator,
2684+
CxxRecordSemantics({parent, Impl.SwiftContext}), {});
2685+
if (semanticsKind == CxxRecordSemanticsKind::MissingLifetimeOperation ||
2686+
semanticsKind == CxxRecordSemanticsKind::UnsafeLifetimeOperation)
26392687
return nullptr;
26402688
}
26412689

@@ -2712,19 +2760,33 @@ namespace {
27122760

27132761
bool foreignReferenceTypePassedByRef(const clang::FunctionDecl *decl) {
27142762
bool anyParamPassesByVal =
2715-
llvm::any_of(decl->parameters(), [this](auto *param) {
2763+
llvm::any_of(decl->parameters(), [this, decl](auto *param) {
27162764
if (auto recordType = dyn_cast<clang::RecordType>(
2717-
param->getType().getCanonicalType()))
2718-
return recordHasReferenceSemantics(recordType->getDecl());
2765+
param->getType().getCanonicalType())) {
2766+
if (recordHasReferenceSemantics(recordType->getDecl())) {
2767+
Impl.addImportDiagnostic(
2768+
decl,
2769+
Diagnostic(diag::reference_passed_by_value,
2770+
Impl.SwiftContext.AllocateCopy(recordType->getDecl()->getNameAsString()),
2771+
"a parameter"));
2772+
return true;
2773+
}
2774+
}
27192775
return false;
27202776
});
27212777

27222778
if (anyParamPassesByVal)
27232779
return true;
27242780

27252781
if (auto recordType = dyn_cast<clang::RecordType>(
2726-
decl->getReturnType().getCanonicalType()))
2782+
decl->getReturnType().getCanonicalType())) {
2783+
Impl.addImportDiagnostic(
2784+
decl,
2785+
Diagnostic(diag::reference_passed_by_value,
2786+
Impl.SwiftContext.AllocateCopy(recordType->getDecl()->getNameAsString()),
2787+
"the return"));
27272788
return recordHasReferenceSemantics(recordType->getDecl());
2789+
}
27282790

27292791
return false;
27302792
}

‎lib/ClangImporter/ImportName.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1482,7 +1482,8 @@ static StringRef renameUnsafeMethod(ASTContext &ctx,
14821482
const clang::NamedDecl *decl,
14831483
StringRef name) {
14841484
if (isa<clang::CXXMethodDecl>(decl) &&
1485-
!evaluateOrDefault(ctx.evaluator, IsSafeUseOfCxxDecl({decl}), {})) {
1485+
!evaluateOrDefault(ctx.evaluator,
1486+
IsSafeUseOfCxxDecl({decl, ctx}), {})) {
14861487
return ctx.getIdentifier(("__" + name + "Unsafe").str()).str();
14871488
}
14881489

‎lib/ClangImporter/SwiftLookupTable.cpp

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,30 +1869,6 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
18691869
if (shouldSuppressDeclImport(named))
18701870
return;
18711871

1872-
// Leave incomplete struct/enum/union types out of the table; Swift only
1873-
// handles pointers to them.
1874-
// FIXME: At some point we probably want to be importing incomplete types,
1875-
// so that pointers to different incomplete types themselves have distinct
1876-
// types. At that time it will be necessary to make the decision of whether
1877-
// or not to import an incomplete type declaration based on whether it's
1878-
// actually the struct backing a CF type:
1879-
//
1880-
// typedef struct CGColor *CGColorRef;
1881-
//
1882-
// The best way to do this is probably to change CFDatabase.def to include
1883-
// struct names when relevant, not just pointer names. That way we can check
1884-
// both CFDatabase.def and the objc_bridge attribute and cover all our bases.
1885-
if (auto *tagDecl = dyn_cast<clang::TagDecl>(named)) {
1886-
// We add entries for ClassTemplateSpecializations that don't have
1887-
// definition. It's possible that the decl will be instantiated by
1888-
// SwiftDeclConverter later on. We cannot force instantiating
1889-
// ClassTemplateSPecializations here because we're currently writing the
1890-
// AST, so we cannot modify it.
1891-
if (!isa<clang::ClassTemplateSpecializationDecl>(named) &&
1892-
!tagDecl->getDefinition()) {
1893-
return;
1894-
}
1895-
}
18961872
// If we have a name to import as, add this entry to the table.
18971873
auto currentVersion =
18981874
ImportNameVersion::fromOptions(nameImporter.getLangOpts());

0 commit comments

Comments
 (0)