Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ namespace swift {
class FuncDecl;
class SourceManager;
class SourceFile;
class ParamDecl;
class AnyPattern;

/// Enumeration describing all of possible diagnostics.
///
Expand Down
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticGroups.def
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ GROUP(WeakMutability, "weak-mutability")
GROUP(PerformanceHints, "performance-hints")
GROUP(ReturnTypeImplicitCopy, "return-type-implicit-copy")
GROUP_LINK(PerformanceHints, ReturnTypeImplicitCopy)
GROUP(ExistentialAnyType, "existential-any-type")
GROUP_LINK(PerformanceHints, ExistentialAnyType)

#define UNDEFINE_DIAGNOSTIC_GROUPS_MACROS
#include "swift/AST/DefineDiagnosticGroupsMacros.h"
24 changes: 24 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -8946,6 +8946,30 @@ GROUPED_WARNING(perf_hint_closure_returns_array,ReturnTypeImplicitCopy,DefaultIg
"Performance: closure returns a%select{ dictionary|n array}0, leading to implicit copies. "
"Consider using an 'inout' parameter instead.", (bool))

GROUPED_WARNING(perf_hint_param_expects_existential_any,ExistentialAnyType,DefaultIgnore,
"Performance: %0 expects existential any, leading to heap allocation, reference counting, "
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
(const ParamDecl*))
GROUPED_WARNING(perf_hint_func_returns_existential_any,ExistentialAnyType,DefaultIgnore,
"Performance: %0 returns existential any, leading to heap allocation, reference counting, "
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
(const FuncDecl*))
GROUPED_WARNING(perf_hint_closure_returns_existential_any,ExistentialAnyType,DefaultIgnore,
"Performance: closure returns existential any, leading to heap allocation, reference counting, "
"and dynamic dispatch. Consider using generic constraints or concrete types instead.", ())
GROUPED_WARNING(perf_hint_var_uses_existential_any,ExistentialAnyType,DefaultIgnore,
"Performance: %0 uses existential any, leading to heap allocation, reference counting, "
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
(const VarDecl *))
GROUPED_WARNING(perf_hint_any_pattern_uses_existential_any,ExistentialAnyType,DefaultIgnore,
"Performance: Ignored value uses existential any, leading to heap allocation, reference counting, "
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
())
GROUPED_WARNING(perf_hint_typealias_uses_existential_any,ExistentialAnyType,DefaultIgnore,
"Performance: %0 aliases existential any type, leading to heap allocation, reference counting, "
"and dynamic dispatch. Consider using generic constraints or concrete types instead.",
(const TypeAliasDecl *))

ERROR(unsafe_self_dependent_result_attr_on_invalid_decl,none,
"invalid use of @_unsafeSelfDependentResult", ())

Expand Down
152 changes: 148 additions & 4 deletions lib/Sema/PerformanceHints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,27 @@
#include "swift/AST/Evaluator.h"
#include "swift/AST/Expr.h"
#include "swift/AST/TypeCheckRequests.h"
#include "swift/AST/TypeVisitor.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);
return !ctx.Diags.isIgnoredDiagnostic(
diag::perf_hint_closure_returns_array.ID) ||
!ctx.Diags.isIgnoredDiagnostic(
diag::perf_hint_function_returns_array.ID) ||
!ctx.Diags.isIgnoredDiagnostic(
diag::perf_hint_param_expects_existential_any.ID) ||
!ctx.Diags.isIgnoredDiagnostic(
diag::perf_hint_func_returns_existential_any.ID) ||
!ctx.Diags.isIgnoredDiagnostic(
diag::perf_hint_closure_returns_existential_any.ID) ||
!ctx.Diags.isIgnoredDiagnostic(
diag::perf_hint_var_uses_existential_any.ID) ||
!ctx.Diags.isIgnoredDiagnostic(
diag::perf_hint_any_pattern_uses_existential_any.ID) ||
!ctx.Diags.isIgnoredDiagnostic(
diag::perf_hint_typealias_uses_existential_any.ID);
}

namespace {
Expand All @@ -52,6 +67,88 @@ void checkImplicitCopyReturnType(const ClosureExpr *Closure,
}
}

class CheckExistentialAny : public TypeVisitor<CheckExistentialAny, bool> {
public:
static bool inType(Type type) {
return CheckExistentialAny().visit(type->getCanonicalType());
}

static void inFunctionReturnType(FuncDecl *FD, DiagnosticEngine &Diags) {
Type T = FD->getResultInterfaceType();

if (inType(T))
Diags.diagnose(FD, diag::perf_hint_func_returns_existential_any, FD);
}

static void inClosureReturnType(ClosureExpr *CE, DiagnosticEngine &Diags) {
Type T = CE->getResultType();

if (inType(T))
Diags.diagnose(CE->getLoc(),
diag::perf_hint_closure_returns_existential_any);
}

static void inVariableType(const VarDecl *VD, DiagnosticEngine &Diags) {
Type T = VD->getInterfaceType();

if (inType(T))
Diags.diagnose(VD, diag::perf_hint_var_uses_existential_any, VD);
}

static void inPatternType(const AnyPattern *AP, DiagnosticEngine &Diags) {
Type T = AP->getType();

if (inType(T))
Diags.diagnose(AP->getLoc(),
diag::perf_hint_any_pattern_uses_existential_any);
}

static void inTypeAlias(const TypeAliasDecl *TAD, DiagnosticEngine &Diags) {
Type T = TAD->getUnderlyingType();

if (inType(T))
Diags.diagnose(TAD->getLoc(),
diag::perf_hint_typealias_uses_existential_any, TAD);
}

bool visitExistentialType(ExistentialType *ET) {
return true;
}

bool visitTupleType(TupleType *TT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Using TypeVisitor for this is incorrect because you're going to miss a lot of cases. For example, a nominal type might have a parent, which you're not visiting at all:

struct Outer<T> {
  struct Inner {}
}

Outer<any P>.Inner

Please use Type.findIf() or Type.visit() instead.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in #8ab27f Thanks for the example, added it as a test case.

for (const auto &element : TT->getElements()) {
if (visit(element.getType())) {
return true;
}
}
return false;
}

bool visitBoundGenericType(BoundGenericType *BGT) {
// Check generic arguments (e.g., Array<any Protocol>)
for (Type arg : BGT->getGenericArgs()) {
if (visit(arg)) {
return true;
}
}
return false;
}

bool visitFunctionType(FunctionType *FT) {
for (const auto &param : FT->getParams()) {
if (visit(param.getPlainType()->getCanonicalType())) {
return true;
}
}

return visit(FT->getResult()->getCanonicalType());
}

bool visitType(TypeBase *T) {
return false;
}
};

/// Produce performance hint diagnostics for a SourceFile.
class PerformanceHintDiagnosticWalker final : public ASTWalker {
ASTContext &Ctx;
Expand All @@ -64,16 +161,63 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
SF->walk(Walker);
}

PreWalkResult<Pattern *> walkToPatternPre(Pattern *P) override {
if (P->isImplicit())
return Action::SkipNode(P);

if (const AnyPattern *AP = dyn_cast<AnyPattern>(P)) {
CheckExistentialAny::inPatternType(AP, Ctx.Diags);
}

return Action::Continue(P);
}

PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
if (auto Closure = dyn_cast<ClosureExpr>(E))
if (E->isImplicit())
return Action::SkipNode(E);

if (const ClosureExpr* Closure = dyn_cast<ClosureExpr>(E)) {
checkImplicitCopyReturnType(Closure, Ctx.Diags);
}

return Action::Continue(E);
}

PostWalkResult<Expr *> walkToExprPost(Expr *E) override {
assert(
!E->isImplicit() &&
"Traversing implicit expressions is disabled in the pre-walk visitor");

if (auto Closure = dyn_cast<ClosureExpr>(E)) {
CheckExistentialAny::inClosureReturnType(Closure, Ctx.Diags);
}

return Action::Continue(E);
}

PreWalkAction walkToDeclPre(Decl *D) override {
if (auto *FD = dyn_cast<FuncDecl>(D))
if (D->isImplicit())
return Action::SkipNode();

if (const FuncDecl *FD = dyn_cast<FuncDecl>(D)) {
checkImplicitCopyReturnType(FD, Ctx.Diags);
} else if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
CheckExistentialAny::inVariableType(VD, Ctx.Diags);
} else if (const TypeAliasDecl *TAD = dyn_cast<TypeAliasDecl>(D)) {
CheckExistentialAny::inTypeAlias(TAD, Ctx.Diags);
}

return Action::Continue();
}

PostWalkAction walkToDeclPost(Decl *D) override {
assert(
!D->isImplicit() &&
"Traversing implicit declarations is disabled in the pre-walk visitor");

if (auto *FD = dyn_cast<FuncDecl>(D)) {
CheckExistentialAny::inFunctionReturnType(FD, Ctx.Diags);
}

return Action::Continue();
}
Expand Down
Loading