Skip to content

Commit d053a90

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 d053a90

File tree

12 files changed

+163
-3
lines changed

12 files changed

+163
-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/PerformanceHints.cpp

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
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 "swift/AST/ASTContext.h"
20+
#include "swift/AST/ASTWalker.h"
21+
#include "swift/AST/Decl.h"
22+
#include "swift/AST/DiagnosticsFrontend.h"
23+
#include "swift/AST/DiagnosticsSema.h"
24+
#include "swift/AST/Evaluator.h"
25+
#include "swift/AST/Expr.h"
26+
#include "swift/AST/TypeCheckRequests.h"
27+
28+
using namespace swift;
29+
30+
namespace {
31+
32+
void checkImplicitCopyReturnType(const FuncDecl *FD, DiagnosticEngine &Diags) {
33+
auto ReturnType = FD->getResultInterfaceType();
34+
if (ReturnType->isArray() || ReturnType->isDictionary()) {
35+
Diags.diagnose(FD->getLoc(), diag::perf_hint_function_returns_array, FD,
36+
ReturnType->isArray());
37+
}
38+
}
39+
40+
void checkImplicitCopyReturnType(const ClosureExpr *Closure,
41+
DiagnosticEngine &Diags) {
42+
auto ReturnType = Closure->getResultType();
43+
if (ReturnType->isArray() || ReturnType->isDictionary()) {
44+
Diags.diagnose(Closure->getLoc(), diag::perf_hint_closure_returns_array,
45+
ReturnType->isArray());
46+
}
47+
}
48+
49+
/// Produce performance hint diagnostics for a SourceFile.
50+
class PerformanceHintDiagnosticWalker final : public ASTWalker {
51+
ASTContext &Ctx;
52+
53+
public:
54+
PerformanceHintDiagnosticWalker(ASTContext &Ctx) : Ctx(Ctx) {}
55+
56+
static void check(SourceFile *SF) {
57+
auto Walker = PerformanceHintDiagnosticWalker(SF->getASTContext());
58+
SF->walk(Walker);
59+
}
60+
61+
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
62+
if (auto Closure = dyn_cast<ClosureExpr>(E))
63+
checkImplicitCopyReturnType(Closure, Ctx.Diags);
64+
65+
return Action::Continue(E);
66+
}
67+
68+
PreWalkAction walkToDeclPre(Decl *D) override {
69+
if (auto *FD = dyn_cast<FuncDecl>(D))
70+
checkImplicitCopyReturnType(FD, Ctx.Diags);
71+
72+
return Action::Continue();
73+
}
74+
};
75+
} // namespace
76+
77+
evaluator::SideEffect EmitPerformanceHints::evaluate(Evaluator &evaluator,
78+
SourceFile *SF) const {
79+
PerformanceHintDiagnosticWalker::check(SF);
80+
return {};
81+
}

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+
// TODO: Only enable when specific checks are enabled
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);
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: %empty-directory(%t)
2+
3+
// Ensure diagnosed as error by-default
4+
// RUN: not %target-swift-frontend -typecheck %s -diagnostic-style llvm &> %t/output_err.txt
5+
// RUN: cat %t/output_err.txt | %FileCheck %s
6+
7+
// Ensure ignored by-default
8+
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -verify
9+
10+
// Ensure enabled with '-Wwarning'
11+
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -Wwarning PerfHintReturnTypeImplicitCopy &> %t/output_warn.txt
12+
// RUN: cat %t/output_warn.txt | %FileCheck %s -check-prefix CHECK-WARN
13+
14+
// Ensure escalated with '-Werror'
15+
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -Werror PerfHintReturnTypeImplicitCopy &> %t/output_err.txt
16+
// RUN: cat %t/output_err.txt | %FileCheck %s -check-prefix CHECK-ERR
17+
18+
// CHECK-ERR: error: Performance: 'foo()' returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#PerfHintReturnTypeImplicitCopy]
19+
// CHECK-ERR: error: Performance: closure returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#PerfHintReturnTypeImplicitCopy]
20+
21+
// CHECK-WARN: warning: Performance: 'foo()' returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#PerfHintReturnTypeImplicitCopy]
22+
// CHECK-WARN: warning: Performance: closure returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#PerfHintReturnTypeImplicitCopy]
23+
24+
func foo() -> [Int] {
25+
return [1,2,3,4,5,6]
26+
}
27+
28+
func bar() -> Int {
29+
let myCoolArray = { () -> [Int] in
30+
return [1, 2]
31+
}
32+
return myCoolArray().count
33+
}

0 commit comments

Comments
 (0)