Skip to content

Commit ef738e6

Browse files
beccadaxbeccadax
committed
Make @abi type diagnostics more specific
Specify whether the type with the problem is a result type, parameter type (and which parameter), etc.
1 parent 7703d11 commit ef738e6

File tree

3 files changed

+280
-221
lines changed

3 files changed

+280
-221
lines changed

‎include/swift/AST/DiagnosticsSema.def‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8397,9 +8397,11 @@ NOTE(attr_abi_matching_attr_here,none,
83978397
"%select{|implicitly added }2here",
83988398
(/*matches=*/bool, /*isModifier=*/bool, /*isImplicit=*/bool))
83998399

8400+
#define TYPE_ORIGIN(KIND_IDX, DECL_IDX) "%select{|%kind" #DECL_IDX " |" \
8401+
"self parameter |result |thrown |%error}" #KIND_IDX
84008402
ERROR(attr_abi_mismatched_type,none,
8401-
"type %0 in '@abi' should match %1",
8402-
(Type, Type))
8403+
TYPE_ORIGIN(0, 1) "type %2 in '@abi' should match %3",
8404+
(unsigned, Decl *, Type, Type))
84038405
NOTE(attr_abi_should_match_type_here,none,
84048406
"should match type here", ())
84058407

@@ -8416,9 +8418,10 @@ ERROR(attr_abi_extra_generic_signature,none,
84168418
(Decl *))
84178419

84188420
ERROR(attr_abi_mismatched_param_modifier,none,
8419-
"%select{default |}0%3 %select{attribute|modifier}2 "
8420-
"%select{|'%0' }0in '@abi' is not compatible with %select{default|'%1'}1",
8421-
(StringRef, StringRef, /*isModifier=*/bool, DescriptiveDeclKind))
8421+
"%select{default |}0%select{attribute|modifier}2 %select{|'%0' }0"
8422+
"on " TYPE_ORIGIN(3, 4) "in '@abi' is not compatible with "
8423+
"%select{default|'%1'}1",
8424+
(StringRef, StringRef, /*isModifier=*/bool, unsigned, Decl *))
84228425
ERROR(attr_abi_no_default_arguments,none,
84238426
"%kind0 in '@abi' should not have a default argument; it does not "
84248427
"affect the parameter's ABI",

‎lib/Sema/TypeCheckAttrABI.cpp‎

Lines changed: 70 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,54 @@ struct DeclEffects {
9090
}
9191
};
9292

93+
/// Describes the relationship between a given type and the declaration it
94+
/// belongs to--e.g. is this its result type? a parameter type? etc. Used with
95+
/// a couple of \c \@abi diagnostics.
96+
class TypeOrigin {
97+
public:
98+
// Cases must be kept in sync with DiagnosticsSema TYPE_ORIGIN
99+
enum class Kind : uint8_t {
100+
Unspecified = 0,
101+
Parameter = 1,
102+
SelfParameter = 2,
103+
Result = 3,
104+
ThrowsEffect = 4,
105+
};
106+
107+
private:
108+
llvm::PointerIntPair<Decl *, 3, Kind> declAndKind;
109+
110+
TypeOrigin(Decl *decl, Kind kind)
111+
: declAndKind(decl, kind) {}
112+
113+
public:
114+
static TypeOrigin forUnspecified() {
115+
return TypeOrigin(nullptr, Kind::Unspecified);
116+
}
117+
118+
static TypeOrigin forParameter(ParamDecl *paramDecl) {
119+
return TypeOrigin(paramDecl,
120+
paramDecl->isSelfParameter() ? Kind::SelfParameter
121+
: Kind::Parameter);
122+
}
123+
124+
static TypeOrigin forResult() {
125+
return TypeOrigin(nullptr, Kind::Result);
126+
}
127+
128+
static TypeOrigin forThrowsEffect() {
129+
return TypeOrigin(nullptr, Kind::ThrowsEffect);
130+
}
131+
132+
Kind getKind() const {
133+
return declAndKind.getInt();
134+
}
135+
136+
Decl *getDecl() const {
137+
return declAndKind.getPointer();
138+
}
139+
};
140+
93141
/// Emit a fix-it replacing \p charRange with \p newText , inserting or
94142
/// removing whitespace after \c charRange in a way suitable for editing a
95143
/// sequence of whitespce-separated keywords.
@@ -217,8 +265,10 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
217265
ParameterTypeFlags abiOrig,
218266
Type apiType, Type abiType,
219267
SourceLoc apiTypeLoc, SourceLoc abiTypeLoc,
220-
DescriptiveDeclKind declKind,
221-
bool isSelfParam) {
268+
TypeOrigin origin) {
269+
// Some keywords are spelled differently for a `self` parameter.
270+
bool isSelfParam = origin.getKind() == TypeOrigin::Kind::SelfParameter;
271+
222272
bool didDiagnose = false;
223273

224274
auto noteShouldMatch = [&](bool isModifier) {
@@ -255,7 +305,8 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
255305
ctx.Diags.diagnose(abiTypeLoc, diag::attr_abi_mismatched_param_modifier,
256306
getSpelling(abiOrig.getOwnershipSpecifier()),
257307
getSpelling(apiOrig.getOwnershipSpecifier()),
258-
/*isModifier=*/true, declKind);
308+
/*isModifier=*/true, unsigned(origin.getKind()),
309+
origin.getDecl());
259310
noteShouldMatch(/*isModifier=*/true);
260311
didDiagnose = true;
261312
}
@@ -264,7 +315,8 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
264315
ctx.Diags.diagnose(abiTypeLoc, diag::attr_abi_mismatched_param_modifier,
265316
abiOrig.isNoDerivative() ? "noDerivative" : "",
266317
apiOrig.isNoDerivative() ? "noDerivative" : "",
267-
/*isModifier=*/false, declKind);
318+
/*isModifier=*/false, unsigned(origin.getKind()),
319+
origin.getDecl());
268320
noteShouldMatch(/*isModifier=*/false);
269321
didDiagnose = true;
270322
}
@@ -274,14 +326,16 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
274326
ctx.Diags.diagnose(abiTypeLoc, diag::attr_abi_mismatched_param_modifier,
275327
abiOrig.isAddressable() ? spelling : "",
276328
apiOrig.isAddressable() ? spelling : "",
277-
/*isModifier=*/false, declKind);
329+
/*isModifier=*/false, unsigned(origin.getKind()),
330+
origin.getDecl());
278331
noteShouldMatch(/*isModifier=*/false);
279332
didDiagnose = true;
280333
}
281334

282335
if (!didDiagnose && api != abi) {
283336
// Flag difference not otherwise diagnosed. This is a fallback diagnostic.
284337
ctx.Diags.diagnose(abiTypeLoc, diag::attr_abi_mismatched_type,
338+
unsigned(origin.getKind()), origin.getDecl(),
285339
abiType, apiType);
286340
ctx.Diags.diagnose(apiTypeLoc, diag::attr_abi_should_match_type_here);
287341
didDiagnose = true;
@@ -316,10 +370,8 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
316370
SourceLoc abiTypeLoc = getTypeLoc(abi, abiDecl);
317371

318372
didDiagnose |= checkType(apiNorm.getPlainType(), abiNorm.getPlainType(),
319-
apiTypeLoc, abiTypeLoc);
320-
321-
auto declKind = api->isSelfParameter() ? apiDecl->getDescriptiveKind()
322-
: DescriptiveDeclKind::Param;
373+
apiTypeLoc, abiTypeLoc,
374+
TypeOrigin::forParameter(abi));
323375

324376
didDiagnose |= checkParameterFlags(apiNorm.getParameterFlags(),
325377
abiNorm.getParameterFlags(),
@@ -328,7 +380,7 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
328380
apiNorm.getPlainType(),
329381
abiNorm.getPlainType(),
330382
apiTypeLoc, abiTypeLoc,
331-
declKind, api->isSelfParameter());
383+
TypeOrigin::forParameter(abi));
332384

333385
didDiagnose |= checkAttrs(api->getAttrs(), abi->getAttrs(), api, abi);
334386

@@ -642,7 +694,8 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
642694
return checkType(api->getResultInterfaceType(),
643695
abi->getResultInterfaceType(),
644696
api->getResultTypeSourceRange().Start,
645-
abi->getResultTypeSourceRange().Start);
697+
abi->getResultTypeSourceRange().Start,
698+
TypeOrigin::forResult());
646699
}
647700

648701
bool visitConstructorDecl(ConstructorDecl *api, ConstructorDecl *abi) {
@@ -658,7 +711,8 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
658711
return true;
659712

660713
if (checkType(api->getValueInterfaceType(), abi->getValueInterfaceType(),
661-
getTypeLoc(api), getTypeLoc(abi)))
714+
getTypeLoc(api), getTypeLoc(abi),
715+
TypeOrigin::forUnspecified()))
662716
return true;
663717

664718
return false;
@@ -868,7 +922,8 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
868922

869923
// MARK: @abi checking - types
870924

871-
bool checkType(Type api, Type abi, SourceLoc apiLoc, SourceLoc abiLoc) {
925+
bool checkType(Type api, Type abi, SourceLoc apiLoc, SourceLoc abiLoc,
926+
TypeOrigin origin) {
872927
if (!api.isNull() && !abi.isNull()) {
873928
Type apiNorm = normalizeType(api);
874929
Type abiNorm = normalizeType(abi);
@@ -877,7 +932,8 @@ class ABIDeclChecker : public ASTComparisonVisitor<ABIDeclChecker> {
877932
}
878933
}
879934

880-
ctx.Diags.diagnose(abiLoc, diag::attr_abi_mismatched_type, abi, api);
935+
ctx.Diags.diagnose(abiLoc, diag::attr_abi_mismatched_type,
936+
unsigned(origin.getKind()), origin.getDecl(), abi, api);
881937
ctx.Diags.diagnose(apiLoc, diag::attr_abi_should_match_type_here);
882938
return true;
883939
}

0 commit comments

Comments
 (0)