Skip to content

Commit 3caa3e3

Browse files
committed
[Performance Hints] Issue diagnostic for code wrapped in implicit nodes
In order to avoid issuing diagnostics for compiler-generate code, we ignore the syntax subtree rooted at an implicit node. This causes our algorithm to miss user code wrapped in implicit nodes during desugaring. For example, expressions in implicit returns from single-expression closures and trailing closures are completely ignored right now. This change fixes heproblem by introducing a simple change to the AST walker. Instead of ignoring the entire implicit-node subtree, we only ignore the implicit node but continue to descend into its children. This enables our algorithm to access non-implicit (user code) nodes nested inside implicit (compiler-generated) nodes.
1 parent d54f601 commit 3caa3e3

File tree

2 files changed

+26
-31
lines changed

2 files changed

+26
-31
lines changed

lib/Sema/PerformanceHints.cpp

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -125,16 +125,9 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
125125
SF->walk(Walker);
126126
}
127127

128-
PreWalkResult<Pattern *> walkToPatternPre(Pattern *P) override {
129-
if (P->isImplicit())
130-
return Action::SkipNode(P);
131-
132-
return Action::Continue(P);
133-
}
134-
135128
PostWalkResult<Pattern *> walkToPatternPost(Pattern *P) override {
136-
assert(!P->isImplicit() &&
137-
"Traversing implicit patterns is disabled in the pre-walk visitor");
129+
if (P->isImplicit())
130+
return Action::Continue(P);
138131

139132
if (const AnyPattern *AP = dyn_cast<AnyPattern>(P)) {
140133
checkExistentialInPatternType(AP, Ctx.Diags);
@@ -143,17 +136,9 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
143136
return Action::Continue(P);
144137
}
145138

146-
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
147-
if (E->isImplicit())
148-
return Action::SkipNode(E);
149-
150-
return Action::Continue(E);
151-
}
152-
153139
PostWalkResult<Expr *> walkToExprPost(Expr *E) override {
154-
assert(
155-
!E->isImplicit() &&
156-
"Traversing implicit expressions is disabled in the pre-walk visitor");
140+
if (E->isImplicit())
141+
return Action::Continue(E);
157142

158143
if (const ClosureExpr *CE = dyn_cast<ClosureExpr>(E)) {
159144
checkImplicitCopyReturnType(CE, Ctx.Diags);
@@ -163,17 +148,9 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
163148
return Action::Continue(E);
164149
}
165150

166-
PreWalkAction walkToDeclPre(Decl *D) override {
167-
if (D->isImplicit())
168-
return Action::SkipNode();
169-
170-
return Action::Continue();
171-
}
172-
173151
PostWalkAction walkToDeclPost(Decl *D) override {
174-
assert(
175-
!D->isImplicit() &&
176-
"Traversing implicit declarations is disabled in the pre-walk visitor");
152+
if (D->isImplicit())
153+
return Action::Continue();
177154

178155
if (const FuncDecl *FD = dyn_cast<FuncDecl>(D)) {
179156
checkImplicitCopyReturnType(FD, Ctx.Diags);

test/PerformanceHints/existential-any.swift

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func animalParam1(_ animal: any Animal) {} // expected-error {{Performance: 'an
7171
// Multiple parameters
7272
func animalParam2(
7373
_ animal: any Animal, // expected-error {{Performance: 'animal' uses an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
74-
to other: any Animal // expected-error {{Performance: 'other' uses an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
74+
to other: any Animal // expected-error {{Performance: 'other' uses an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
7575
) {}
7676

7777
// Variadic parameters
@@ -144,7 +144,7 @@ func tupleTest() {
144144

145145
let (
146146
animalsA, // expected-error {{Performance: 'animalsA' uses an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
147-
animalsB // expected-error {{Performance: 'animalsB' uses an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
147+
animalsB // expected-error {{Performance: 'animalsB' uses an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
148148
) = ([Tiger() as any Animal], [Panda() as any Animal])
149149
print(type(of: animalsA))
150150
print(type(of: animalsB))
@@ -244,3 +244,21 @@ struct Outer<T> {
244244
func f() -> Outer<any Animal>.Inner { // expected-error {{Performance: 'f()' returns an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
245245
return Outer<any Animal>().i
246246
}
247+
248+
////////////////////////////////////////////////////////////////////////////////
249+
// Implicit AST Nodes
250+
////////////////////////////////////////////////////////////////////////////////
251+
252+
func stringPlusInReduce() {
253+
let animalKinds = ["Tiger", "Panda", "Tiger", "Dodo"]
254+
let animals: [any Animal] = // expected-error {{Performance: 'animals' uses an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
255+
animalKinds.map { animalKind in // expected-error {{Performance: closure returns an existential, leading to heap allocation, reference counting, and dynamic dispatch. Consider using generic constraints or concrete types instead.}}
256+
switch animalKind {
257+
case "Tiger":
258+
return Tiger()
259+
default:
260+
return Panda()
261+
}
262+
}
263+
print(animals)
264+
}

0 commit comments

Comments
 (0)