https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/109145
This patch essentially reverts #108674 while adding a testcase that triggers a crash in clang-tidy. Fixes #108963. >From c230f7e30e60f11c5675ec1dd9f49f5f6378dc6f Mon Sep 17 00:00:00 2001 From: Kazu Hirata <k...@google.com> Date: Wed, 18 Sep 2024 00:19:25 -0700 Subject: [PATCH] [clang-tidy] Fix performance-unnecessary-value-param This patch essentially reverts #108674 while adding a testcase that triggers a crash in clang-tidy. Fixes #108963. --- .../unnecessary-value-param-crash.cpp | 23 +++++++++++++++++++ .../Analysis/Analyses/ExprMutationAnalyzer.h | 17 ++++++++++---- 2 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp new file mode 100644 index 00000000000000..99c2fe905bdf37 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp @@ -0,0 +1,23 @@ +// RUN: %check_clang_tidy -std=c++14-or-later %s performance-unnecessary-value-param %t + +// The test case used to crash clang-tidy. +// https://github.com/llvm/llvm-project/issues/108963 + +struct A +{ + template<typename T> A(T&&) {} +}; + +struct B +{ + ~B(); +}; + +struct C +{ + A a; + C(B, int i) : a(i) {} + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: the parameter #1 is copied for each invocation but only used as a const reference; consider making it a const reference +}; + +C c(B(), 0); diff --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h index b7b84852168e2e..d7d0d80ee8cd8c 100644 --- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h +++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h @@ -118,10 +118,19 @@ class FunctionParmMutationAnalyzer { static FunctionParmMutationAnalyzer * getFunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context, ExprMutationAnalyzer::Memoized &Memorized) { - auto [it, Inserted] = Memorized.FuncParmAnalyzer.try_emplace(&Func); - if (Inserted) - it->second = std::unique_ptr<FunctionParmMutationAnalyzer>( - new FunctionParmMutationAnalyzer(Func, Context, Memorized)); + auto it = Memorized.FuncParmAnalyzer.find(&Func); + if (it == Memorized.FuncParmAnalyzer.end()) + // We call try_emplace here, repeating a hash lookup on the same key. Note + // that creating a new instance of FunctionParmMutationAnalyzer below may + // add additional elements to FuncParmAnalyzer and invalidate iterators. + // That means that we cannot call try_emplace above and update the value + // portion (i.e. it->second) here. + it = + Memorized.FuncParmAnalyzer + .try_emplace(&Func, std::unique_ptr<FunctionParmMutationAnalyzer>( + new FunctionParmMutationAnalyzer( + Func, Context, Memorized))) + .first; return it->getSecond().get(); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
