Skip to content

Commit 146e95c

Browse files
hamishknighthamishknight
committed
[Sema] Use BaseDiagnosticWalker consistently in MiscDiagnostics
None of these walkers ought to be walking into non-PatternBinding decls, this avoids duplicate diagnostics in nested local decls in closures.
1 parent fab09c5 commit 146e95c

File tree

2 files changed

+20
-70
lines changed

2 files changed

+20
-70
lines changed

‎lib/Sema/MiscDiagnostics.cpp‎

Lines changed: 10 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,7 +1561,7 @@ static void diagRecursivePropertyAccess(const Expr *E, const DeclContext *DC) {
15611561
if (!var) // Ignore subscripts
15621562
return;
15631563

1564-
class DiagnoseWalker : public ASTWalker {
1564+
class DiagnoseWalker : public BaseDiagnosticWalker {
15651565
ASTContext &Ctx;
15661566
VarDecl *Var;
15671567
const AccessorDecl *Accessor;
@@ -1577,12 +1577,6 @@ static void diagRecursivePropertyAccess(const Expr *E, const DeclContext *DC) {
15771577
cast<VarDecl>(DRE->getDecl())->isSelfParameter();
15781578
}
15791579

1580-
MacroWalking getMacroWalkingBehavior() const override {
1581-
// Macro expansions will be walked when they're type-checked, not as
1582-
// part of the surrounding code.
1583-
return MacroWalking::None;
1584-
}
1585-
15861580
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
15871581
Expr *subExpr;
15881582
bool isStore = false;
@@ -4548,7 +4542,7 @@ static void checkStmtConditionTrailingClosure(ASTContext &ctx, const Expr *E) {
45484542
if (E == nullptr || isa<ErrorExpr>(E)) return;
45494543

45504544
// Walk into expressions which might have invalid trailing closures
4551-
class DiagnoseWalker : public ASTWalker {
4545+
class DiagnoseWalker : public BaseDiagnosticWalker {
45524546
ASTContext &Ctx;
45534547

45544548
void diagnoseIt(const CallExpr *E) {
@@ -4594,12 +4588,6 @@ static void checkStmtConditionTrailingClosure(ASTContext &ctx, const Expr *E) {
45944588
public:
45954589
DiagnoseWalker(ASTContext &ctx) : Ctx(ctx) { }
45964590

4597-
MacroWalking getMacroWalkingBehavior() const override {
4598-
// Macro expansions will be walked when they're type-checked, not as
4599-
// part of the surrounding code.
4600-
return MacroWalking::None;
4601-
}
4602-
46034591
PreWalkResult<ArgumentList *>
46044592
walkToArgumentListPre(ArgumentList *args) override {
46054593
// Don't walk into an explicit argument list, as trailing closures that
@@ -4669,7 +4657,7 @@ static void checkStmtConditionTrailingClosure(ASTContext &ctx, const Stmt *S) {
46694657

46704658
namespace {
46714659

4672-
class ObjCSelectorWalker : public ASTWalker {
4660+
class ObjCSelectorWalker : public BaseDiagnosticWalker {
46734661
ASTContext &Ctx;
46744662
const DeclContext *DC;
46754663
Type SelectorTy;
@@ -4719,12 +4707,6 @@ class ObjCSelectorWalker : public ASTWalker {
47194707
ObjCSelectorWalker(const DeclContext *dc, Type selectorTy)
47204708
: Ctx(dc->getASTContext()), DC(dc), SelectorTy(selectorTy) { }
47214709

4722-
MacroWalking getMacroWalkingBehavior() const override {
4723-
// Macro expansions will be walked when they're type-checked, not as
4724-
// part of the surrounding code.
4725-
return MacroWalking::None;
4726-
}
4727-
47284710
PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
47294711
auto *stringLiteral = dyn_cast<StringLiteralExpr>(expr);
47304712
bool fromStringLiteral = false;
@@ -5231,7 +5213,7 @@ static void diagnoseUnintendedOptionalBehavior(const Expr *E,
52315213
if (!E || isa<ErrorExpr>(E) || !E->getType())
52325214
return;
52335215

5234-
class UnintendedOptionalBehaviorWalker : public ASTWalker {
5216+
class UnintendedOptionalBehaviorWalker : public BaseDiagnosticWalker {
52355217
ASTContext &Ctx;
52365218
SmallPtrSet<Expr *, 16> IgnoredExprs;
52375219

@@ -5583,12 +5565,6 @@ static void diagnoseUnintendedOptionalBehavior(const Expr *E,
55835565
}
55845566
}
55855567

5586-
MacroWalking getMacroWalkingBehavior() const override {
5587-
// Macro expansions will be walked when they're type-checked, not as
5588-
// part of the surrounding code.
5589-
return MacroWalking::None;
5590-
}
5591-
55925568
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
55935569
if (!E || isa<ErrorExpr>(E) || !E->getType())
55945570
return Action::SkipNode(E);
@@ -5624,7 +5600,7 @@ static void diagnoseDeprecatedWritableKeyPath(const Expr *E,
56245600
if (!E || isa<ErrorExpr>(E) || !E->getType())
56255601
return;
56265602

5627-
class DeprecatedWritableKeyPathWalker : public ASTWalker {
5603+
class DeprecatedWritableKeyPathWalker : public BaseDiagnosticWalker {
56285604
ASTContext &Ctx;
56295605
const DeclContext *DC;
56305606

@@ -5658,12 +5634,6 @@ static void diagnoseDeprecatedWritableKeyPath(const Expr *E,
56585634
}
56595635
}
56605636

5661-
MacroWalking getMacroWalkingBehavior() const override {
5662-
// Macro expansions will be walked when they're type-checked, not as
5663-
// part of the surrounding code.
5664-
return MacroWalking::None;
5665-
}
5666-
56675637
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
56685638
if (!E || isa<ErrorExpr>(E) || !E->getType())
56695639
return Action::SkipNode(E);
@@ -5687,7 +5657,7 @@ static void diagnoseDeprecatedWritableKeyPath(const Expr *E,
56875657

56885658
static void maybeDiagnoseCallToKeyValueObserveMethod(const Expr *E,
56895659
const DeclContext *DC) {
5690-
class KVOObserveCallWalker : public ASTWalker {
5660+
class KVOObserveCallWalker : public BaseDiagnosticWalker {
56915661
const ASTContext &C;
56925662

56935663
public:
@@ -5745,12 +5715,6 @@ static void maybeDiagnoseCallToKeyValueObserveMethod(const Expr *E,
57455715
}
57465716
}
57475717

5748-
MacroWalking getMacroWalkingBehavior() const override {
5749-
// Macro expansions will be walked when they're type-checked, not as
5750-
// part of the surrounding code.
5751-
return MacroWalking::None;
5752-
}
5753-
57545718
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
57555719
if (!E || isa<ErrorExpr>(E) || !E->getType())
57565720
return Action::SkipNode(E);
@@ -5771,7 +5735,7 @@ static void maybeDiagnoseCallToKeyValueObserveMethod(const Expr *E,
57715735
static void diagnoseExplicitUseOfLazyVariableStorage(const Expr *E,
57725736
const DeclContext *DC) {
57735737

5774-
class ExplicitLazyVarStorageAccessFinder : public ASTWalker {
5738+
class ExplicitLazyVarStorageAccessFinder : public BaseDiagnosticWalker {
57755739
const ASTContext &C;
57765740

57775741
public:
@@ -5798,12 +5762,6 @@ static void diagnoseExplicitUseOfLazyVariableStorage(const Expr *E,
57985762
}
57995763
}
58005764

5801-
MacroWalking getMacroWalkingBehavior() const override {
5802-
// Macro expansions will be walked when they're type-checked, not as
5803-
// part of the surrounding code.
5804-
return MacroWalking::None;
5805-
}
5806-
58075765
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
58085766
if (!E || isa<ErrorExpr>(E) || !E->getType())
58095767
return Action::SkipNode(E);
@@ -5822,7 +5780,7 @@ static void diagnoseExplicitUseOfLazyVariableStorage(const Expr *E,
58225780
}
58235781

58245782
static void diagnoseComparisonWithNaN(const Expr *E, const DeclContext *DC) {
5825-
class ComparisonWithNaNFinder : public ASTWalker {
5783+
class ComparisonWithNaNFinder : public BaseDiagnosticWalker {
58265784
const ASTContext &C;
58275785

58285786
public:
@@ -5931,12 +5889,6 @@ static void diagnoseComparisonWithNaN(const Expr *E, const DeclContext *DC) {
59315889
}
59325890
}
59335891

5934-
MacroWalking getMacroWalkingBehavior() const override {
5935-
// Macro expansions will be walked when they're type-checked, not as
5936-
// part of the surrounding code.
5937-
return MacroWalking::None;
5938-
}
5939-
59405892
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
59415893
if (!E || isa<ErrorExpr>(E) || !E->getType())
59425894
return Action::SkipNode(E);
@@ -5959,19 +5911,13 @@ static void diagUnqualifiedAccessToMethodNamedSelf(const Expr *E,
59595911
if (!E || isa<ErrorExpr>(E) || !E->getType())
59605912
return;
59615913

5962-
class DiagnoseWalker : public ASTWalker {
5914+
class DiagnoseWalker : public BaseDiagnosticWalker {
59635915
ASTContext &Ctx;
59645916
const DeclContext *DC;
59655917

59665918
public:
59675919
DiagnoseWalker(const DeclContext *DC) : Ctx(DC->getASTContext()), DC(DC) {}
59685920

5969-
MacroWalking getMacroWalkingBehavior() const override {
5970-
// Macro expansions will be walked when they're type-checked, not as
5971-
// part of the surrounding code.
5972-
return MacroWalking::None;
5973-
}
5974-
59755921
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
59765922
if (!E || isa<ErrorExpr>(E) || !E->getType())
59775923
return Action::SkipNode(E);
@@ -6024,7 +5970,7 @@ static void diagUnqualifiedAccessToMethodNamedSelf(const Expr *E,
60245970
static void
60255971
diagnoseDictionaryLiteralDuplicateKeyEntries(const Expr *E,
60265972
const DeclContext *DC) {
6027-
class DiagnoseWalker : public ASTWalker {
5973+
class DiagnoseWalker : public BaseDiagnosticWalker {
60285974
ASTContext &Ctx;
60295975

60305976
private:
@@ -6123,12 +6069,6 @@ diagnoseDictionaryLiteralDuplicateKeyEntries(const Expr *E,
61236069
public:
61246070
DiagnoseWalker(const DeclContext *DC) : Ctx(DC->getASTContext()) {}
61256071

6126-
MacroWalking getMacroWalkingBehavior() const override {
6127-
// Macro expansions will be walked when they're type-checked, not as
6128-
// part of the surrounding code.
6129-
return MacroWalking::None;
6130-
}
6131-
61326072
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
61336073
const auto *DLE = dyn_cast_or_null<DictionaryExpr>(E);
61346074
if (!DLE)

‎test/Sema/diag_unintended_optional_behavior.swift‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,3 +224,13 @@ func warnOptionalInStringInterpolationSegment(_ o : Int?) {
224224
// expected-note@-2 {{use 'String(describing:)' to silence this warning}} {{51-51=String(describing: }} {{55-55=)}}
225225
// expected-note@-3 {{provide a default value to avoid this warning}} {{55-55= ?? <#default value#>}}
226226
}
227+
228+
// Make sure we don't double diagnose
229+
_ = {
230+
func foo(_ y: Int?) {
231+
let _: Any = y // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}}
232+
// expected-note@-1 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}
233+
// expected-note@-2 {{provide a default value to avoid this warning}}
234+
// expected-note@-3 {{force-unwrap the value to avoid this warning}}
235+
}
236+
}

0 commit comments

Comments
 (0)