Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticGroups.def
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,9 @@ GROUP(UnknownWarningGroup, "unknown-warning-group")
GROUP(CompilationCaching, "compilation-caching")
GROUP(WeakMutability, "weak-mutability")

GROUP(PerformanceHints, "performance-hints")
GROUP(ReturnTypeImplicitCopy, "return-type-implicit-copy")
GROUP_LINK(PerformanceHints, ReturnTypeImplicitCopy)

#define UNDEFINE_DIAGNOSTIC_GROUPS_MACROS
#include "swift/AST/DefineDiagnosticGroupsMacros.h"
11 changes: 11 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -8935,5 +8935,16 @@ ERROR(attr_inline_always_requires_inlinable,none,
ERROR(attr_inline_always_no_usable_from_inline,none,
"cannot use '@inline(always)' together with '@usableFromInline'", ())

//===----------------------------------------------------------------------===//
// MARK: Swift Performance hints
//===----------------------------------------------------------------------===//

GROUPED_WARNING(perf_hint_function_returns_array,ReturnTypeImplicitCopy,DefaultIgnore,
"Performance: %0 returns a%select{ dictionary|n array}1, leading to implicit copies. "
"Consider using an 'inout' parameter instead.", (const FuncDecl *, bool))
GROUPED_WARNING(perf_hint_closure_returns_array,ReturnTypeImplicitCopy,DefaultIgnore,
"Performance: closure returns a%select{ dictionary|n array}0, leading to implicit copies. "
"Consider using an 'inout' parameter instead.", (bool))

#define UNDEFINE_DIAGNOSTIC_MACROS
#include "DefineDiagnosticMacros.h"
18 changes: 18 additions & 0 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -4967,6 +4967,24 @@ class TypeCheckCDeclFunctionRequest
bool isCached() const { return true; }
};

/// A request to emit performance hints
class EmitPerformanceHints
: public SimpleRequest<EmitPerformanceHints,
evaluator::SideEffect(SourceFile *),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

evaluator::SideEffect
evaluate(Evaluator &evaluator, SourceFile *SF) const;

public:
bool isCached() const { return true; }
};

/// Check @c enums for compatibility with C.
class TypeCheckCDeclEnumRequest
: public SimpleRequest<TypeCheckCDeclEnumRequest,
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -664,3 +664,7 @@ SWIFT_REQUEST(TypeChecker, AvailabilityDomainForDeclRequest,
SWIFT_REQUEST(TypeChecker, IsCustomAvailabilityDomainPermanentlyEnabled,
bool(const CustomAvailabilityDomain *),
SeparatelyCached, NoLocationInfo)

SWIFT_REQUEST(TypeChecker, EmitPerformanceHints,
evaluator::SideEffect(SourceFile *),
Cached, NoLocationInfo)
1 change: 1 addition & 0 deletions lib/Sema/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ add_swift_host_library(swiftSema STATIC
MiscDiagnostics.cpp
OpenedExistentials.cpp
PCMacro.cpp
PerformanceHints.cpp
PlaygroundTransform.cpp
PreCheckTarget.cpp
ResilienceDiagnostics.cpp
Expand Down
6 changes: 3 additions & 3 deletions lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3227,7 +3227,7 @@ bool swift::computeFixitsForOverriddenDeclaration(
}

//===----------------------------------------------------------------------===//
// Per func/init diagnostics
// MARK: Per func/init diagnostics
//===----------------------------------------------------------------------===//

namespace {
Expand Down Expand Up @@ -6474,7 +6474,7 @@ static void diagnoseCxxFunctionCalls(const Expr *E, const DeclContext *DC) {
}

//===----------------------------------------------------------------------===//
// High-level entry points.
// MARK: High-level entry points.
//===----------------------------------------------------------------------===//

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

//===----------------------------------------------------------------------===//
// Utility functions
// MARK: Utility functions
//===----------------------------------------------------------------------===//

void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
Expand Down
3 changes: 3 additions & 0 deletions lib/Sema/MiscDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ namespace swift {
bool diagnoseUnhandledThrowsInAsyncContext(DeclContext *dc,
ForEachStmt *forEach);

/// Determine if any of the performance hint diagnostics are enabled.
bool performanceHintDiagnosticsEnabled(ASTContext &ctx);

class BaseDiagnosticWalker : public ASTWalker {
protected:
PreWalkAction walkToDeclPre(Decl *D) override {
Expand Down
87 changes: 87 additions & 0 deletions lib/Sema/PerformanceHints.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//===--------------------- PerformanceHints.cpp ---------------------------===//
//
// This source file is part of the Swift.org open source project

//
// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//
//
// This files implements the various checks/lints intended to provide
// opt-in guidance for hidden costs in performance-critical Swift code.
//
//===----------------------------------------------------------------------===//

#include "MiscDiagnostics.h"
#include "swift/AST/ASTContext.h"
#include "swift/AST/ASTWalker.h"
#include "swift/AST/Decl.h"
#include "swift/AST/DiagnosticsFrontend.h"
#include "swift/AST/DiagnosticsSema.h"
#include "swift/AST/Evaluator.h"
#include "swift/AST/Expr.h"
#include "swift/AST/TypeCheckRequests.h"

using namespace swift;

bool swift::performanceHintDiagnosticsEnabled(ASTContext &ctx) {
return !ctx.Diags.isIgnoredDiagnostic(diag::perf_hint_closure_returns_array.ID) ||
!ctx.Diags.isIgnoredDiagnostic(diag::perf_hint_function_returns_array.ID);
Comment on lines +32 to +33
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is the best we have right now. At some point (not for this PR!) we should introduce a group-level query for this, which probably means a different underlying data structure.

}

namespace {

void checkImplicitCopyReturnType(const FuncDecl *FD, DiagnosticEngine &Diags) {
auto ReturnType = FD->getResultInterfaceType();
if (ReturnType->isArray() || ReturnType->isDictionary()) {
Diags.diagnose(FD->getLoc(), diag::perf_hint_function_returns_array, FD,
ReturnType->isArray());
}
}

void checkImplicitCopyReturnType(const ClosureExpr *Closure,
DiagnosticEngine &Diags) {
auto ReturnType = Closure->getResultType();
if (ReturnType->isArray() || ReturnType->isDictionary()) {
Diags.diagnose(Closure->getLoc(), diag::perf_hint_closure_returns_array,
ReturnType->isArray());
}
}

/// Produce performance hint diagnostics for a SourceFile.
class PerformanceHintDiagnosticWalker final : public ASTWalker {
ASTContext &Ctx;

public:
PerformanceHintDiagnosticWalker(ASTContext &Ctx) : Ctx(Ctx) {}

static void check(SourceFile *SF) {
auto Walker = PerformanceHintDiagnosticWalker(SF->getASTContext());
SF->walk(Walker);
}

PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
if (auto Closure = dyn_cast<ClosureExpr>(E))
checkImplicitCopyReturnType(Closure, Ctx.Diags);

return Action::Continue(E);
}

PreWalkAction walkToDeclPre(Decl *D) override {
if (auto *FD = dyn_cast<FuncDecl>(D))
checkImplicitCopyReturnType(FD, Ctx.Diags);

return Action::Continue();
}
};
} // namespace

evaluator::SideEffect EmitPerformanceHints::evaluate(Evaluator &evaluator,
SourceFile *SF) const {
PerformanceHintDiagnosticWalker::check(SF);
return {};
}
4 changes: 4 additions & 0 deletions lib/Sema/TypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,10 @@ TypeCheckPrimaryFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
Ctx.evaluator,
CheckInconsistentWeakLinkedImportsRequest{SF->getParentModule()}, {});

// Opt-in performance hint diagnostics for performance-critical code.
if (performanceHintDiagnosticsEnabled(Ctx))
evaluateOrDefault(Ctx.evaluator, EmitPerformanceHints{SF}, {});

// Perform various AST transforms we've been asked to perform.
if (!Ctx.hadError() && Ctx.LangOpts.DebuggerTestingTransform)
performDebuggerTestingTransform(*SF);
Expand Down
39 changes: 39 additions & 0 deletions test/PerformanceHints/array-ret.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// RUN: %empty-directory(%t)

// Ensure ignored by-default
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -verify

// Ensure enabled with '-Wwarning'
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -Wwarning PerformanceHints &> %t/output_warn.txt
// RUN: cat %t/output_warn.txt | %FileCheck %s -check-prefix CHECK-WARN
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -Wwarning ReturnTypeImplicitCopy &> %t/output_warn.txt
// RUN: cat %t/output_warn.txt | %FileCheck %s -check-prefix CHECK-WARN

// Ensure enabled with '-Werror' for the broad category and downgraded to warning for the subgroup with '-Wwarning'
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -Werror PerformanceHints -Wwarning ReturnTypeImplicitCopy &> %t/output_warn.txt
// RUN: cat %t/output_warn.txt | %FileCheck %s -check-prefix CHECK-WARN

// Ensure enabled with '-Wwarning' for the broad category and downgraded to warning for the subgroup with '-Werror'
// RUN: not %target-swift-frontend -typecheck %s -diagnostic-style llvm -Wwarning PerformanceHints -Werror ReturnTypeImplicitCopy &> %t/output_err.txt
// RUN: cat %t/output_err.txt | %FileCheck %s -check-prefix CHECK-ERR

// Ensure escalated with '-Werror'
// RUN: not %target-swift-frontend -typecheck %s -diagnostic-style llvm -Werror ReturnTypeImplicitCopy &> %t/output_err.txt
// RUN: cat %t/output_err.txt | %FileCheck %s -check-prefix CHECK-ERR

// CHECK-ERR: error: Performance: 'foo()' returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#ReturnTypeImplicitCopy]
// CHECK-ERR: error: Performance: closure returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#ReturnTypeImplicitCopy]

// CHECK-WARN: warning: Performance: 'foo()' returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#ReturnTypeImplicitCopy]
// CHECK-WARN: warning: Performance: closure returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#ReturnTypeImplicitCopy]

func foo() -> [Int] {
return [1,2,3,4,5,6]
}

func bar() -> Int {
let myCoolArray = { () -> [Int] in
return [1, 2]
}
return myCoolArray().count
}
1 change: 1 addition & 0 deletions userdocs/diagnostics/diagnostic-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,4 @@ presented specially within your IDE of choice. See below for the full list of th
- <doc:opaque-type-inference>
- <doc:mutable-global-variable>
- <doc:existential-member-access-limitations>
- <doc:performance-hints>
3 changes: 3 additions & 0 deletions userdocs/diagnostics/performance-hints.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Swift Performance Hint: Function returning Array or Dictionary type

- TODO: Docs