Skip to content

Commit 1cf692f

Browse files
committed
[Performance Hints] Implement check for existential any
This commit introduces a performance hint check that warns on the use of existential any in variable declarations, function and closure parameters and returns, and typealiases.
1 parent d06d126 commit 1cf692f

File tree

5 files changed

+388
-5
lines changed

5 files changed

+388
-5
lines changed

include/swift/AST/DiagnosticEngine.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ namespace swift {
4848
class FuncDecl;
4949
class SourceManager;
5050
class SourceFile;
51+
class ParamDecl;
52+
class AnyPattern;
5153

5254
/// Enumeration describing all of possible diagnostics.
5355
///

include/swift/AST/DiagnosticGroups.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ GROUP(WeakMutability, "weak-mutability")
8282
GROUP(PerformanceHints, "performance-hints")
8383
GROUP(ReturnTypeImplicitCopy, "return-type-implicit-copy")
8484
GROUP_LINK(PerformanceHints, ReturnTypeImplicitCopy)
85+
GROUP(ExistentialType, "existential-type")
86+
GROUP_LINK(PerformanceHints, ExistentialType)
8587

8688
#define UNDEFINE_DIAGNOSTIC_GROUPS_MACROS
8789
#include "swift/AST/DefineDiagnosticGroupsMacros.h"

include/swift/AST/DiagnosticsSema.def

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8946,6 +8946,30 @@ GROUPED_WARNING(perf_hint_closure_returns_array,ReturnTypeImplicitCopy,DefaultIg
89468946
"Performance: closure returns a%select{ dictionary|n array}0, leading to implicit copies. "
89478947
"Consider using an 'inout' parameter instead.", (bool))
89488948

8949+
GROUPED_WARNING(perf_hint_param_expects_existential,ExistentialType,DefaultIgnore,
8950+
"Performance: %0 expects an existential, leading to heap allocation, reference counting, "
8951+
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
8952+
(const ParamDecl*))
8953+
GROUPED_WARNING(perf_hint_func_returns_existential,ExistentialType,DefaultIgnore,
8954+
"Performance: %0 returns an existential, leading to heap allocation, reference counting, "
8955+
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
8956+
(const FuncDecl*))
8957+
GROUPED_WARNING(perf_hint_closure_returns_existential,ExistentialType,DefaultIgnore,
8958+
"Performance: closure returns an existential, leading to heap allocation, reference counting, "
8959+
"and dynamic dispatch. Consider using generic constraints or concrete types instead.", ())
8960+
GROUPED_WARNING(perf_hint_var_uses_existential,ExistentialType,DefaultIgnore,
8961+
"Performance: %0 uses an existential, leading to heap allocation, reference counting, "
8962+
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
8963+
(const VarDecl *))
8964+
GROUPED_WARNING(perf_hint_any_pattern_uses_existential,ExistentialType,DefaultIgnore,
8965+
"Performance: declaration uses an existential, leading to heap allocation, reference counting, "
8966+
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
8967+
())
8968+
GROUPED_WARNING(perf_hint_typealias_uses_existential,ExistentialType,DefaultIgnore,
8969+
"Performance: %0 aliases an existential type, leading to heap allocation, reference counting, "
8970+
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
8971+
(const TypeAliasDecl *))
8972+
89498973
ERROR(unsafe_self_dependent_result_attr_on_invalid_decl,none,
89508974
"invalid use of @_unsafeSelfDependentResult", ())
89518975

lib/Sema/PerformanceHints.cpp

Lines changed: 114 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,27 @@
2525
#include "swift/AST/Evaluator.h"
2626
#include "swift/AST/Expr.h"
2727
#include "swift/AST/TypeCheckRequests.h"
28+
#include "swift/AST/TypeVisitor.h"
2829

2930
using namespace swift;
3031

3132
bool swift::performanceHintDiagnosticsEnabled(ASTContext &ctx) {
32-
return !ctx.Diags.isIgnoredDiagnostic(diag::perf_hint_closure_returns_array.ID) ||
33-
!ctx.Diags.isIgnoredDiagnostic(diag::perf_hint_function_returns_array.ID);
33+
return !ctx.Diags.isIgnoredDiagnostic(
34+
diag::perf_hint_closure_returns_array.ID) ||
35+
!ctx.Diags.isIgnoredDiagnostic(
36+
diag::perf_hint_function_returns_array.ID) ||
37+
!ctx.Diags.isIgnoredDiagnostic(
38+
diag::perf_hint_param_expects_existential.ID) ||
39+
!ctx.Diags.isIgnoredDiagnostic(
40+
diag::perf_hint_func_returns_existential.ID) ||
41+
!ctx.Diags.isIgnoredDiagnostic(
42+
diag::perf_hint_closure_returns_existential.ID) ||
43+
!ctx.Diags.isIgnoredDiagnostic(
44+
diag::perf_hint_var_uses_existential.ID) ||
45+
!ctx.Diags.isIgnoredDiagnostic(
46+
diag::perf_hint_any_pattern_uses_existential.ID) ||
47+
!ctx.Diags.isIgnoredDiagnostic(
48+
diag::perf_hint_typealias_uses_existential.ID);
3449
}
3550

3651
namespace {
@@ -52,6 +67,52 @@ void checkImplicitCopyReturnType(const ClosureExpr *Closure,
5267
}
5368
}
5469

70+
bool hasExistentialAnyInType(Type T) {
71+
return T->getCanonicalType().findIf(
72+
[](CanType CT) { return isa<ExistentialType>(CT); });
73+
}
74+
75+
void checkExistentialInFunctionReturnType(const FuncDecl *FD,
76+
DiagnosticEngine &Diags) {
77+
Type T = FD->getResultInterfaceType();
78+
79+
if (hasExistentialAnyInType(T))
80+
Diags.diagnose(FD, diag::perf_hint_func_returns_existential, FD);
81+
}
82+
83+
void checkExistentialInClosureReturnType(const ClosureExpr *CE,
84+
DiagnosticEngine &Diags) {
85+
Type T = CE->getResultType();
86+
87+
if (hasExistentialAnyInType(T))
88+
Diags.diagnose(CE->getLoc(), diag::perf_hint_closure_returns_existential);
89+
}
90+
91+
void checkExistentialInVariableType(const VarDecl *VD,
92+
DiagnosticEngine &Diags) {
93+
Type T = VD->getInterfaceType();
94+
95+
if (hasExistentialAnyInType(T))
96+
Diags.diagnose(VD, diag::perf_hint_var_uses_existential, VD);
97+
}
98+
99+
void checkExistentialInPatternType(const AnyPattern *AP,
100+
DiagnosticEngine &Diags) {
101+
Type T = AP->getType();
102+
103+
if (hasExistentialAnyInType(T))
104+
Diags.diagnose(AP->getLoc(), diag::perf_hint_any_pattern_uses_existential);
105+
}
106+
107+
void checkExistentialInTypeAlias(const TypeAliasDecl *TAD,
108+
DiagnosticEngine &Diags) {
109+
Type T = TAD->getUnderlyingType();
110+
111+
if (hasExistentialAnyInType(T))
112+
Diags.diagnose(TAD->getLoc(), diag::perf_hint_typealias_uses_existential,
113+
TAD);
114+
}
115+
55116
/// Produce performance hint diagnostics for a SourceFile.
56117
class PerformanceHintDiagnosticWalker final : public ASTWalker {
57118
ASTContext &Ctx;
@@ -64,16 +125,64 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
64125
SF->walk(Walker);
65126
}
66127

128+
PreWalkResult<Pattern *> walkToPatternPre(Pattern *P) override {
129+
if (P->isImplicit())
130+
return Action::SkipNode(P);
131+
132+
return Action::Continue(P);
133+
}
134+
135+
PostWalkResult<Pattern *> walkToPatternPost(Pattern *P) override {
136+
assert(!P->isImplicit() &&
137+
"Traversing implicit patterns is disabled in the pre-walk visitor");
138+
139+
if (const AnyPattern *AP = dyn_cast<AnyPattern>(P)) {
140+
checkExistentialInPatternType(AP, Ctx.Diags);
141+
}
142+
143+
return Action::Continue(P);
144+
}
145+
67146
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
68-
if (auto Closure = dyn_cast<ClosureExpr>(E))
69-
checkImplicitCopyReturnType(Closure, Ctx.Diags);
147+
if (E->isImplicit())
148+
return Action::SkipNode(E);
149+
150+
return Action::Continue(E);
151+
}
152+
153+
PostWalkResult<Expr *> walkToExprPost(Expr *E) override {
154+
assert(
155+
!E->isImplicit() &&
156+
"Traversing implicit expressions is disabled in the pre-walk visitor");
157+
158+
if (const ClosureExpr *CE = dyn_cast<ClosureExpr>(E)) {
159+
checkImplicitCopyReturnType(CE, Ctx.Diags);
160+
checkExistentialInClosureReturnType(CE, Ctx.Diags);
161+
}
70162

71163
return Action::Continue(E);
72164
}
73165

74166
PreWalkAction walkToDeclPre(Decl *D) override {
75-
if (auto *FD = dyn_cast<FuncDecl>(D))
167+
if (D->isImplicit())
168+
return Action::SkipNode();
169+
170+
return Action::Continue();
171+
}
172+
173+
PostWalkAction walkToDeclPost(Decl *D) override {
174+
assert(
175+
!D->isImplicit() &&
176+
"Traversing implicit declarations is disabled in the pre-walk visitor");
177+
178+
if (const FuncDecl *FD = dyn_cast<FuncDecl>(D)) {
76179
checkImplicitCopyReturnType(FD, Ctx.Diags);
180+
checkExistentialInFunctionReturnType(FD, Ctx.Diags);
181+
} else if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
182+
checkExistentialInVariableType(VD, Ctx.Diags);
183+
} else if (const TypeAliasDecl *TAD = dyn_cast<TypeAliasDecl>(D)) {
184+
checkExistentialInTypeAlias(TAD, Ctx.Diags);
185+
}
77186

78187
return Action::Continue();
79188
}

0 commit comments

Comments
 (0)