Skip to content

Commit 7050c7b

Browse files
committed
[Performance Hints] Add simple check for returning of values of array and dictionary type
This check will run on each type-checked primary input of the current compilation and emit a warning diagnostic for all discovered occurences of this code pattern when the performance hint diagnostic is enabled
1 parent 7853ba0 commit 7050c7b

13 files changed

+168
-3
lines changed

include/swift/AST/DiagnosticGroups.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ GROUP(TrailingClosureMatching, "trailing-closure-matching")
7878
GROUP(UnknownWarningGroup, "unknown-warning-group")
7979
GROUP(CompilationCaching, "compilation-caching")
8080
GROUP(WeakMutability, "weak-mutability")
81+
GROUP(PerfHintReturnTypeImplicitCopy, "perf-hint-return-type-implicit-copy")
8182

8283
#define UNDEFINE_DIAGNOSTIC_GROUPS_MACROS
8384
#include "swift/AST/DefineDiagnosticGroupsMacros.h"

include/swift/AST/DiagnosticsSema.def

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8935,5 +8935,16 @@ ERROR(attr_inline_always_requires_inlinable,none,
89358935
ERROR(attr_inline_always_no_usable_from_inline,none,
89368936
"cannot use '@inline(always)' together with '@usableFromInline'", ())
89378937

8938+
//===----------------------------------------------------------------------===//
8939+
// MARK: Swift Performance hints
8940+
//===----------------------------------------------------------------------===//
8941+
8942+
GROUPED_WARNING(perf_hint_function_returns_array,PerfHintReturnTypeImplicitCopy,DefaultIgnore,
8943+
"Performance: %0 returns a%select{ dictionary|n array}1, leading to implicit copies. "
8944+
"Consider using an 'inout' parameter instead.", (const FuncDecl *, bool))
8945+
GROUPED_WARNING(perf_hint_closure_returns_array,PerfHintReturnTypeImplicitCopy,DefaultIgnore,
8946+
"Performance: closure returns a%select{ dictionary|n array}0, leading to implicit copies. "
8947+
"Consider using an 'inout' parameter instead.", (bool))
8948+
89388949
#define UNDEFINE_DIAGNOSTIC_MACROS
89398950
#include "DefineDiagnosticMacros.h"

include/swift/AST/TypeCheckRequests.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4967,6 +4967,24 @@ class TypeCheckCDeclFunctionRequest
49674967
bool isCached() const { return true; }
49684968
};
49694969

4970+
/// A request to emit performance hints
4971+
class EmitPerformanceHints
4972+
: public SimpleRequest<EmitPerformanceHints,
4973+
evaluator::SideEffect(SourceFile *),
4974+
RequestFlags::Cached> {
4975+
public:
4976+
using SimpleRequest::SimpleRequest;
4977+
4978+
private:
4979+
friend SimpleRequest;
4980+
4981+
evaluator::SideEffect
4982+
evaluate(Evaluator &evaluator, SourceFile *SF) const;
4983+
4984+
public:
4985+
bool isCached() const { return true; }
4986+
};
4987+
49704988
/// Check @c enums for compatibility with C.
49714989
class TypeCheckCDeclEnumRequest
49724990
: public SimpleRequest<TypeCheckCDeclEnumRequest,

include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,3 +664,7 @@ SWIFT_REQUEST(TypeChecker, AvailabilityDomainForDeclRequest,
664664
SWIFT_REQUEST(TypeChecker, IsCustomAvailabilityDomainPermanentlyEnabled,
665665
bool(const CustomAvailabilityDomain *),
666666
SeparatelyCached, NoLocationInfo)
667+
668+
SWIFT_REQUEST(TypeChecker, EmitPerformanceHints,
669+
evaluator::SideEffect(SourceFile *),
670+
Cached, NoLocationInfo)

lib/Option/features.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@
5353
},
5454
{
5555
"name": "internal-import-bridging-header"
56+
},
57+
{
58+
"name": "performance-hints"
5659
}
5760
]
5861
}

lib/Sema/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ add_swift_host_library(swiftSema STATIC
4444
MiscDiagnostics.cpp
4545
OpenedExistentials.cpp
4646
PCMacro.cpp
47+
PerformanceHints.cpp
4748
PlaygroundTransform.cpp
4849
PreCheckTarget.cpp
4950
ResilienceDiagnostics.cpp

lib/Sema/MiscDiagnostics.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3227,7 +3227,7 @@ bool swift::computeFixitsForOverriddenDeclaration(
32273227
}
32283228

32293229
//===----------------------------------------------------------------------===//
3230-
// Per func/init diagnostics
3230+
// MARK: Per func/init diagnostics
32313231
//===----------------------------------------------------------------------===//
32323232

32333233
namespace {
@@ -6474,7 +6474,7 @@ static void diagnoseCxxFunctionCalls(const Expr *E, const DeclContext *DC) {
64746474
}
64756475

64766476
//===----------------------------------------------------------------------===//
6477-
// High-level entry points.
6477+
// MARK: High-level entry points.
64786478
//===----------------------------------------------------------------------===//
64796479

64806480
/// Emit diagnostics for syntactic restrictions on a given expression.
@@ -6525,7 +6525,7 @@ void swift::performStmtDiagnostics(const Stmt *S, DeclContext *DC) {
65256525
}
65266526

65276527
//===----------------------------------------------------------------------===//
6528-
// Utility functions
6528+
// MARK: Utility functions
65296529
//===----------------------------------------------------------------------===//
65306530

65316531
void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,

lib/Sema/MiscDiagnostics.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ namespace swift {
140140
bool diagnoseUnhandledThrowsInAsyncContext(DeclContext *dc,
141141
ForEachStmt *forEach);
142142

143+
/// Determine if any of the performance hint diagnostics are enabled.
144+
bool performanceHintDiagnosticsEnabled(ASTContext &ctx);
145+
143146
class BaseDiagnosticWalker : public ASTWalker {
144147
protected:
145148
PreWalkAction walkToDeclPre(Decl *D) override {

lib/Sema/PerformanceHints.cpp

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
//===--------------------- PerformanceHints.cpp ---------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
5+
//
6+
// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors
7+
// Licensed under Apache License v2.0 with Runtime Library Exception
8+
//
9+
// See https://swift.org/LICENSE.txt for license information
10+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
11+
//
12+
//===----------------------------------------------------------------------===//
13+
//
14+
// This files implements the various checks/lints intended to provide
15+
// opt-in guidance for hidden costs in performance-critical Swift code.
16+
//
17+
//===----------------------------------------------------------------------===//
18+
19+
#include "MiscDiagnostics.h"
20+
#include "swift/AST/ASTContext.h"
21+
#include "swift/AST/ASTWalker.h"
22+
#include "swift/AST/Decl.h"
23+
#include "swift/AST/DiagnosticsFrontend.h"
24+
#include "swift/AST/DiagnosticsSema.h"
25+
#include "swift/AST/Evaluator.h"
26+
#include "swift/AST/Expr.h"
27+
#include "swift/AST/TypeCheckRequests.h"
28+
29+
using namespace swift;
30+
31+
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);
34+
}
35+
36+
namespace {
37+
38+
void checkImplicitCopyReturnType(const FuncDecl *FD, DiagnosticEngine &Diags) {
39+
auto ReturnType = FD->getResultInterfaceType();
40+
if (ReturnType->isArray() || ReturnType->isDictionary()) {
41+
Diags.diagnose(FD->getLoc(), diag::perf_hint_function_returns_array, FD,
42+
ReturnType->isArray());
43+
}
44+
}
45+
46+
void checkImplicitCopyReturnType(const ClosureExpr *Closure,
47+
DiagnosticEngine &Diags) {
48+
auto ReturnType = Closure->getResultType();
49+
if (ReturnType->isArray() || ReturnType->isDictionary()) {
50+
Diags.diagnose(Closure->getLoc(), diag::perf_hint_closure_returns_array,
51+
ReturnType->isArray());
52+
}
53+
}
54+
55+
/// Produce performance hint diagnostics for a SourceFile.
56+
class PerformanceHintDiagnosticWalker final : public ASTWalker {
57+
ASTContext &Ctx;
58+
59+
public:
60+
PerformanceHintDiagnosticWalker(ASTContext &Ctx) : Ctx(Ctx) {}
61+
62+
static void check(SourceFile *SF) {
63+
auto Walker = PerformanceHintDiagnosticWalker(SF->getASTContext());
64+
SF->walk(Walker);
65+
}
66+
67+
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
68+
if (auto Closure = dyn_cast<ClosureExpr>(E))
69+
checkImplicitCopyReturnType(Closure, Ctx.Diags);
70+
71+
return Action::Continue(E);
72+
}
73+
74+
PreWalkAction walkToDeclPre(Decl *D) override {
75+
if (auto *FD = dyn_cast<FuncDecl>(D))
76+
checkImplicitCopyReturnType(FD, Ctx.Diags);
77+
78+
return Action::Continue();
79+
}
80+
};
81+
} // namespace
82+
83+
evaluator::SideEffect EmitPerformanceHints::evaluate(Evaluator &evaluator,
84+
SourceFile *SF) const {
85+
PerformanceHintDiagnosticWalker::check(SF);
86+
return {};
87+
}

lib/Sema/TypeChecker.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,10 @@ TypeCheckPrimaryFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
384384
Ctx.evaluator,
385385
CheckInconsistentWeakLinkedImportsRequest{SF->getParentModule()}, {});
386386

387+
// Opt-in performance hint diagnostics for performance-critical code.
388+
if (performanceHintDiagnosticsEnabled(Ctx))
389+
evaluateOrDefault(Ctx.evaluator, EmitPerformanceHints{SF}, {});
390+
387391
// Perform various AST transforms we've been asked to perform.
388392
if (!Ctx.hadError() && Ctx.LangOpts.DebuggerTestingTransform)
389393
performDebuggerTestingTransform(*SF);

0 commit comments

Comments
 (0)