Skip to content

[clang-tidy] performance-unnecessary-copy-initialization: Consider static functions #119974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 12, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,18 @@ AST_MATCHER_FUNCTION_P(StatementMatcher,
hasArgument(0, hasType(ReceiverType)))));
}

AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }

AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningFunctionCall) {
// Only allow initialization of a const reference from a free function if it
// has no arguments. Otherwise it could return an alias to one of its
// arguments and the arguments need to be checked for const use as well.
return callExpr(callee(functionDecl(returns(hasCanonicalType(
matchers::isReferenceToConst())))
.bind(FunctionDeclId)),
argumentCountIs(0), unless(callee(cxxMethodDecl())))
.bind(InitFunctionCallId);
// Only allow initialization of a const reference from a free function or
// static member function if it has no arguments. Otherwise it could return
// an alias to one of its arguments and the arguments need to be checked
// for const use as well.
return callExpr(argumentCountIs(0),
callee(functionDecl(returns(hasCanonicalType(matchers::isReferenceToConst())),
unless(cxxMethodDecl(unless(isStatic()))))
.bind(FunctionDeclId)))
.bind(InitFunctionCallId);
}

AST_MATCHER_FUNCTION_P(StatementMatcher, initializerReturnsReferenceToConst,
Expand Down Expand Up @@ -232,7 +235,7 @@ UnnecessaryCopyInitialization::UnnecessaryCopyInitialization(
Options.get("ExcludedContainerTypes", ""))) {}

void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
auto LocalVarCopiedFrom = [this](const internal::Matcher<Expr> &CopyCtorArg) {
auto LocalVarCopiedFrom = [this](const ast_matchers::internal::Matcher<Expr> &CopyCtorArg) {
return compoundStmt(
forEachDescendant(
declStmt(
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,10 @@ Changes in existing checks
<clang-tidy/checks/performance/move-const-arg>` check to fix a crash when
an argument type is declared but not defined.

- Improved :doc:`performance-unnecessary-copy-initialization`
<clang-tidy/checks/performance/unnecessary-copy-initialization> check
to consider static member functions the same way as free functions.

- Improved :doc:`readability-container-contains
<clang-tidy/checks/readability/container-contains>` check to let it work on
any class that has a ``contains`` method. Fix some false negatives in the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ struct ExpensiveToCopyType {
template <typename A>
const A &templatedAccessor() const;
operator int() const; // Implicit conversion to int.

static const ExpensiveToCopyType &instance();
};

template <typename T>
Expand Down Expand Up @@ -100,6 +102,28 @@ void PositiveFunctionCall() {
VarCopyConstructed.constMethod();
}

void PositiveStaticMethodCall() {
const auto AutoAssigned = ExpensiveToCopyType::instance();
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-FIXES: const auto& AutoAssigned = ExpensiveToCopyType::instance();
AutoAssigned.constMethod();

const auto AutoCopyConstructed(ExpensiveToCopyType::instance());
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
// CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveToCopyType::instance());
AutoCopyConstructed.constMethod();

const ExpensiveToCopyType VarAssigned = ExpensiveToCopyType::instance();
// CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
// CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = ExpensiveToCopyType::instance();
VarAssigned.constMethod();

const ExpensiveToCopyType VarCopyConstructed(ExpensiveToCopyType::instance());
// CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
// CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveToCopyType::instance());
VarCopyConstructed.constMethod();
}

void PositiveMethodCallConstReferenceParam(const ExpensiveToCopyType &Obj) {
const auto AutoAssigned = Obj.reference();
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
Expand Down
Loading