Skip to content

Commit 5406f1a

Browse files
zoecarverzoecarver
authored
Merge pull request #63673 from zoecarver/fixits-for-common-swiftifications
[cxx-interop] Offer fix-it to re-write ".at(42)" as "[42]".
2 parents 5a3220e + 38658a0 commit 5406f1a

15 files changed

+394
-147
lines changed

‎include/swift/AST/DiagnosticsClangImporter.def

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -147,12 +147,12 @@ NOTE(macro_not_imported_invalid_numeric_literal, none, "invalid numeric literal"
147147
NOTE(macro_not_imported_unsupported_literal, none, "only numeric and string macro literals supported", ())
148148
NOTE(macro_not_imported_nested_cast, none, "non-null nested casts not supported", ())
149149

150-
NOTE(macro_not_imported_function_like, none, "macro '%0' not imported: function like macros not supported", (StringRef))
151-
NOTE(macro_not_imported_unsupported_structure, none, "macro '%0' not imported: structure not supported", (StringRef))
152-
NOTE(macro_not_imported, none, "macro '%0' not imported", (StringRef))
150+
NOTE(macro_not_imported_function_like, none, "macro '%0' unavailable: function like macros not supported", (StringRef))
151+
NOTE(macro_not_imported_unsupported_structure, none, "macro '%0' unavailable: structure not supported", (StringRef))
152+
NOTE(macro_not_imported, none, "macro '%0' unavailable (cannot import)", (StringRef))
153153

154-
NOTE(return_type_not_imported, none, "return type not imported", ())
155-
NOTE(parameter_type_not_imported, none, "parameter %0 not imported", (const clang::NamedDecl*))
154+
NOTE(return_type_not_imported, none, "return type unavailable (cannot import)", ())
155+
NOTE(parameter_type_not_imported, none, "parameter %0 unavailable (cannot import)", (const clang::NamedDecl*))
156156
NOTE(incomplete_interface, none, "interface %0 is incomplete", (const clang::NamedDecl*))
157157
NOTE(incomplete_protocol, none, "protocol %0 is incomplete", (const clang::NamedDecl*))
158158
NOTE(incomplete_record, none, "record '%0' is not defined (incomplete)", (StringRef))
@@ -170,13 +170,44 @@ NOTE(record_not_automatically_importable, none, "record '%0' is not "
170170
"Refer to the C++ Interop User "
171171
"Manual to classify this type.",
172172
(StringRef, StringRef))
173-
NOTE(projection_not_imported, none, "C++ method '%0' that returns unsafe "
174-
"projection of type '%1' not imported",
173+
174+
NOTE(projection_value_not_imported, none, "C++ method '%0' that returns a value "
175+
"of type '%1' is unavailable. ",
176+
(StringRef, StringRef))
177+
NOTE(projection_ptr_not_imported, none, "C++ method '%0' that returns a pointer "
178+
"of type '%1' is unavailable. ",
175179
(StringRef, StringRef))
176-
NOTE(dont_use_iterator_api, none, "C++ method '%0' that returns an unsafe "
177-
"iterator not imported: use Swift Sequence "
178-
"APIs instead",
180+
NOTE(projection_reference_not_imported, none, "C++ method '%0' that returns a reference "
181+
"of type '%1' is unavailable. ",
182+
(StringRef, StringRef))
183+
NOTE(projection_may_return_interior_ptr, none, "C++ method '%0' may return an "
184+
"interior pointer. ",
185+
(StringRef))
186+
NOTE(mark_self_contained, none, "Mark type '%0' as 'self_contained' in C++ to "
187+
"make methods that use it available in Swift. ",
188+
(StringRef))
189+
NOTE(mark_safe_to_import, none, "Mark method '%0' as 'safe_to_import' in C++ to "
190+
"make it available in Swift. ",
191+
(StringRef))
192+
193+
NOTE(at_to_subscript, none, "Do you want to replace it with a call "
194+
"to the subscript operator?",
195+
())
196+
NOTE(get_swift_iterator, none, "Do you want to make a Swift iterator instead?",
197+
())
198+
NOTE(replace_with_nil, none, "Do you want to compare against 'nil' instead?'",
199+
())
200+
NOTE(get_first_element, none, "Do you want to get the first element instead?",
201+
())
202+
NOTE(get_last_element, none, "Do you want to get the last element instead?",
203+
())
204+
205+
NOTE(iterator_method_unavailable, none, "C++ method '%0' that returns an "
206+
"iterator is unavailable",
179207
(StringRef))
208+
NOTE(iterator_potentially_unsafe, none, "C++ methods that return iterators "
209+
"are potentially unsafe. Try re-writing "
210+
"to use Swift iterator APIs.", ())
180211

181212
ERROR(reference_type_must_have_retain_attr,none,
182213
"reference type '%0' must have 'retain:' swift attribute.", (StringRef))
@@ -210,10 +241,10 @@ ERROR(conforms_to_not_protocol,none,
210241
"specified protocol conformance '%0' is not a protocol.", (StringRef))
211242

212243
NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
213-
NOTE(record_field_not_imported, none, "field %0 not imported", (const clang::NamedDecl*))
214-
NOTE(invoked_func_not_imported, none, "function %0 not imported", (const clang::NamedDecl*))
215-
NOTE(record_method_not_imported, none, "method %0 not imported", (const clang::NamedDecl*))
216-
NOTE(objc_property_not_imported, none, "property %0 not imported", (const clang::NamedDecl*))
244+
NOTE(record_field_not_imported, none, "field %0 unavailable (cannot import)", (const clang::NamedDecl*))
245+
NOTE(invoked_func_not_imported, none, "function %0 unavailable (cannot import)", (const clang::NamedDecl*))
246+
NOTE(record_method_not_imported, none, "method %0 unavailable (cannot import)", (const clang::NamedDecl*))
247+
NOTE(objc_property_not_imported, none, "property %0 unavailable (cannot import)", (const clang::NamedDecl*))
217248

218249
NOTE(forward_declared_interface_label, none, "interface %0 forward declared here", (const clang::NamedDecl*))
219250
NOTE(forward_declared_protocol_label, none, "protocol %0 forward declared here", (const clang::NamedDecl*))

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 128 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3774,8 +3774,10 @@ bool SubscriptMisuseFailure::diagnoseAsNote() {
37743774
return false;
37753775
}
37763776

3777-
static void diagnoseUnsafeCxxMethod(SourceLoc loc, Type baseType,
3778-
DeclName name) {
3777+
void MissingMemberFailure::diagnoseUnsafeCxxMethod(SourceLoc loc,
3778+
ASTNode anchor,
3779+
Type baseType,
3780+
DeclName name) const {
37793781
auto &ctx = baseType->getASTContext();
37803782

37813783
if (baseType->getAnyNominal() == nullptr ||
@@ -3793,25 +3795,131 @@ static void diagnoseUnsafeCxxMethod(SourceLoc loc, Type baseType,
37933795
if (!cxxMethod)
37943796
continue;
37953797

3796-
if (name.getBaseIdentifier().str() == "begin" ||
3797-
name.getBaseIdentifier().str() == "end") {
3798-
ctx.Diags.diagnose(loc, diag::dont_use_iterator_api,
3798+
auto conformsToRACollection = [&](auto baseType) {
3799+
auto raCollectionProto =
3800+
ctx.getProtocol(KnownProtocolKind::CxxRandomAccessCollection);
3801+
// We didn't load the overlay.
3802+
if (raCollectionProto == nullptr)
3803+
return false;
3804+
3805+
SmallVector<ProtocolConformance *, 2> scratch;
3806+
return baseType->getAnyNominal()->lookupConformance(raCollectionProto,
3807+
scratch);
3808+
};
3809+
3810+
auto returnTypeStr = cast<FuncDecl>(found)
3811+
->getResultInterfaceType()
3812+
->getAnyNominal()
3813+
->getName()
3814+
.str();
3815+
3816+
// Rewrite front() and back() as first and last.
3817+
if ((name.getBaseIdentifier().is("front") ||
3818+
name.getBaseIdentifier().is("back")) &&
3819+
cxxMethod->getReturnType()->isReferenceType() &&
3820+
conformsToRACollection(baseType)) {
3821+
auto dotExpr = getAsExpr<UnresolvedDotExpr>(anchor);
3822+
auto callExpr = getAsExpr<CallExpr>(findParentExpr(dotExpr));
3823+
bool isFront = name.getBaseIdentifier().is("front");
3824+
3825+
ctx.Diags.diagnose(loc, diag::projection_reference_not_imported,
3826+
name.getBaseIdentifier().str(), returnTypeStr);
3827+
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
3828+
name.getBaseIdentifier().str());
3829+
ctx.Diags
3830+
.diagnose(loc,
3831+
isFront ? diag::get_first_element : diag::get_last_element)
3832+
.fixItReplaceChars(
3833+
loc, loc.getAdvancedLoc(name.getBaseIdentifier().str().size()),
3834+
isFront ? "first" : "last")
3835+
.fixItRemove({callExpr->getArgs()->getStartLoc(),
3836+
callExpr->getArgs()->getEndLoc()});
3837+
}
3838+
3839+
// Rewrite begin() as a call to makeIterator() and end as nil.
3840+
if (name.getBaseIdentifier().is("begin")) {
3841+
if (conformsToRACollection(baseType)) {
3842+
ctx.Diags.diagnose(loc, diag::iterator_method_unavailable,
3843+
name.getBaseIdentifier().str());
3844+
ctx.Diags.diagnose(loc, diag::get_swift_iterator)
3845+
.fixItReplaceChars(
3846+
loc, loc.getAdvancedLoc(name.getBaseIdentifier().str().size()),
3847+
"makeIterator");
3848+
} else {
3849+
ctx.Diags.diagnose(loc, diag::iterator_method_unavailable,
3850+
name.getBaseIdentifier().str());
3851+
ctx.Diags.diagnose(loc, diag::iterator_potentially_unsafe);
3852+
}
3853+
} else if (name.getBaseIdentifier().is("end")) {
3854+
if (conformsToRACollection(baseType)) {
3855+
auto dotExpr = getAsExpr<UnresolvedDotExpr>(anchor);
3856+
auto callExpr = getAsExpr<CallExpr>(findParentExpr(dotExpr));
3857+
3858+
ctx.Diags.diagnose(loc, diag::iterator_method_unavailable,
3859+
name.getBaseIdentifier().str());
3860+
ctx.Diags.diagnose(loc, diag::replace_with_nil)
3861+
.fixItReplaceChars(loc, callExpr->getArgs()->getEndLoc(), "nil");
3862+
} else {
3863+
ctx.Diags.diagnose(loc, diag::iterator_method_unavailable,
3864+
name.getBaseIdentifier().str());
3865+
ctx.Diags.diagnose(loc, diag::iterator_potentially_unsafe);
3866+
}
3867+
} else if (cxxMethod->getReturnType()->isPointerType()) {
3868+
ctx.Diags.diagnose(loc, diag::projection_ptr_not_imported,
3869+
name.getBaseIdentifier().str(), returnTypeStr);
3870+
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
37993871
name.getBaseIdentifier().str());
3800-
} else if (cxxMethod->getReturnType()->isPointerType())
3801-
ctx.Diags.diagnose(loc, diag::projection_not_imported,
3802-
name.getBaseIdentifier().str(), "pointer");
3803-
else if (cxxMethod->getReturnType()->isReferenceType())
3804-
ctx.Diags.diagnose(loc, diag::projection_not_imported,
3805-
name.getBaseIdentifier().str(), "reference");
3806-
else if (cxxMethod->getReturnType()->isRecordType()) {
3872+
ctx.Diags.diagnose(loc, diag::mark_safe_to_import,
3873+
name.getBaseIdentifier().str());
3874+
} else if (cxxMethod->getReturnType()->isReferenceType()) {
3875+
// Rewrite a call to .at(42) as a subscript.
3876+
if (name.getBaseIdentifier().is("at") &&
3877+
cxxMethod->getReturnType()->isReferenceType() &&
3878+
conformsToRACollection(baseType)) {
3879+
auto dotExpr = getAsExpr<UnresolvedDotExpr>(anchor);
3880+
auto callExpr = getAsExpr<CallExpr>(findParentExpr(dotExpr));
3881+
3882+
ctx.Diags.diagnose(loc, diag::projection_reference_not_imported,
3883+
name.getBaseIdentifier().str(), returnTypeStr);
3884+
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
3885+
name.getBaseIdentifier().str());
3886+
ctx.Diags.diagnose(loc, diag::at_to_subscript)
3887+
.fixItRemove(
3888+
{dotExpr->getDotLoc(), callExpr->getArgs()->getStartLoc()})
3889+
.fixItReplaceChars(
3890+
callExpr->getArgs()->getStartLoc(),
3891+
callExpr->getArgs()->getStartLoc().getAdvancedLoc(1), "[")
3892+
.fixItReplaceChars(
3893+
callExpr->getArgs()->getEndLoc(),
3894+
callExpr->getArgs()->getEndLoc().getAdvancedLoc(1), "]");
3895+
} else {
3896+
ctx.Diags.diagnose(loc, diag::projection_reference_not_imported,
3897+
name.getBaseIdentifier().str(), returnTypeStr);
3898+
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
3899+
name.getBaseIdentifier().str());
3900+
ctx.Diags.diagnose(loc, diag::mark_safe_to_import,
3901+
name.getBaseIdentifier().str());
3902+
}
3903+
} else if (cxxMethod->getReturnType()->isRecordType()) {
38073904
if (auto cxxRecord = dyn_cast<clang::CXXRecordDecl>(
38083905
cxxMethod->getReturnType()->getAsRecordDecl())) {
3809-
assert(evaluateOrDefault(ctx.evaluator,
3810-
CxxRecordSemantics({cxxRecord, ctx}), {}) ==
3811-
CxxRecordSemanticsKind::UnsafePointerMember);
3812-
ctx.Diags.diagnose(loc, diag::projection_not_imported,
3813-
name.getBaseIdentifier().str(),
3814-
cxxRecord->getNameAsString());
3906+
auto methodSemantics = evaluateOrDefault(
3907+
ctx.evaluator, CxxRecordSemantics({cxxRecord, ctx}), {});
3908+
if (methodSemantics == CxxRecordSemanticsKind::Iterator) {
3909+
ctx.Diags.diagnose(loc, diag::iterator_method_unavailable,
3910+
name.getBaseIdentifier().str());
3911+
ctx.Diags.diagnose(loc, diag::iterator_potentially_unsafe);
3912+
} else {
3913+
assert(methodSemantics ==
3914+
CxxRecordSemanticsKind::UnsafePointerMember);
3915+
ctx.Diags.diagnose(loc, diag::projection_value_not_imported,
3916+
name.getBaseIdentifier().str(), returnTypeStr);
3917+
ctx.Diags.diagnose(loc, diag::projection_may_return_interior_ptr,
3918+
name.getBaseIdentifier().str());
3919+
ctx.Diags.diagnose(loc, diag::mark_safe_to_import,
3920+
name.getBaseIdentifier().str());
3921+
ctx.Diags.diagnose(loc, diag::mark_self_contained, returnTypeStr);
3922+
}
38153923
}
38163924
}
38173925
}
@@ -3891,7 +3999,8 @@ bool MissingMemberFailure::diagnoseAsError() {
38913999
if (!ctx.LangOpts.DisableExperimentalClangImporterDiagnostics) {
38924000
ctx.getClangModuleLoader()->diagnoseMemberValue(getName().getFullName(),
38934001
baseType);
3894-
diagnoseUnsafeCxxMethod(getLoc(), baseType, getName().getFullName());
4002+
diagnoseUnsafeCxxMethod(getLoc(), anchor, baseType,
4003+
getName().getFullName());
38954004
}
38964005
};
38974006

‎lib/Sema/CSDiagnostics.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1255,7 +1255,18 @@ class MissingMemberFailure : public InvalidMemberRefFailure {
12551255
/// overload to be present, but a class marked as `@dynamicCallable`
12561256
/// defines only `dynamicallyCall(withArguments:)` variant.
12571257
bool diagnoseForDynamicCallable() const;
1258-
1258+
1259+
/// Diagnose methods that return unsafe projections and suggest fixits.
1260+
/// For example, if Swift cannot find "vector::data" because it is unsafe, try
1261+
/// to diagnose this and tell the user why we did not import "vector::data".
1262+
///
1263+
/// Provides fixits for:
1264+
/// at -> subscript
1265+
/// begin, end -> makeIterator
1266+
/// front, back -> first, last
1267+
void diagnoseUnsafeCxxMethod(SourceLoc loc, ASTNode anchor, Type baseType,
1268+
DeclName name) const;
1269+
12591270
/// Tailored diagnostics for collection literal with unresolved member expression
12601271
/// that defaults the element type. e.g. _ = [.e]
12611272
bool diagnoseInLiteralCollectionContext() const;

‎test/ClangImporter/experimental_clang_importer_diagnostics_bridging_header.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ s.c = 5
55
// CHECK: experimental_clang_importer_diagnostics_bridging_header.swift:{{[0-9]+}}:3: error: value of type 'PartialImport' has no member 'c'
66
// CHECK-NEXT: s.c = 5
77
// CHECK-NEXT: ~ ^
8-
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:3: note: field 'c' not imported
8+
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:3: note: field 'c' unavailable (cannot import)
99
// CHECK-NEXT: int _Complex c;
1010
// CHECK-NEXT: ^
1111
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:3: note: built-in type 'Complex' not supported
@@ -16,10 +16,10 @@ _ = CFunctionReturningAForwardDeclaredInterface()
1616
// CHECK: experimental_clang_importer_diagnostics_bridging_header.swift:{{[0-9]+}}:5: error: cannot find 'CFunctionReturningAForwardDeclaredInterface' in scope
1717
// CHECK-NEXT: _ = CFunctionReturningAForwardDeclaredInterface()
1818
// CHECK-NEXT: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
19-
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: function 'CFunctionReturningAForwardDeclaredInterface' not imported
19+
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: function 'CFunctionReturningAForwardDeclaredInterface' unavailable (cannot import)
2020
// CHECK-NEXT: ForwardDeclaredInterface* CFunctionReturningAForwardDeclaredInterface();
2121
// CHECK-NEXT: ^
22-
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: return type not imported
22+
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: return type unavailable (cannot import)
2323
// CHECK-NEXT: ForwardDeclaredInterface* CFunctionReturningAForwardDeclaredInterface();
2424
// CHECK-NEXT: ^
2525
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: interface 'ForwardDeclaredInterface' is incomplete
@@ -33,18 +33,18 @@ _ = FUNC_LIKE_MACRO()
3333
// CHECK: experimental_clang_importer_diagnostics_bridging_header.swift:{{[0-9]+}}:{{[0-9]+}}: error: cannot find 'FUNC_LIKE_MACRO' in scope
3434
// CHECK-NEXT: _ = FUNC_LIKE_MACRO()
3535
// CHECK-NEXT: ^~~~~~~~~~~~~~~
36-
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:9: note: macro 'FUNC_LIKE_MACRO' not imported: function like macros not supported
36+
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:9: note: macro 'FUNC_LIKE_MACRO' unavailable: function like macros not supported
3737
// CHECK-NEXT: #define FUNC_LIKE_MACRO() 0
3838
// CHECK-NEXT: {{^}} ^
3939

4040
unsupported_return_type()
4141
// CHECK: experimental_clang_importer_diagnostics_bridging_header.swift:{{[0-9]+}}:{{[0-9]+}}: error: cannot find 'unsupported_return_type' in scope
4242
// CHECK-NEXT: unsupported_return_type()
4343
// CHECK-NEXT: ^~~~~~~~~~~~~~~~~~~~~~~
44-
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: function 'unsupported_return_type' not imported
44+
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: function 'unsupported_return_type' unavailable (cannot import)
4545
// CHECK-NEXT: _Complex int unsupported_return_type();
4646
// CHECK-NEXT: {{^}}^
47-
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: return type not imported
47+
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: return type unavailable (cannot import)
4848
// CHECK-NEXT: _Complex int unsupported_return_type();
4949
// CHECK-NEXT: {{^}}^
5050
// CHECK-NEXT: experimental_clang_importer_diagnostics_bridging_header.h:{{[0-9]+}}:1: note: built-in type 'Complex' not supported

0 commit comments

Comments
 (0)