llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Kazu Hirata (kazutakahirata) <details> <summary>Changes</summary> This patch essentially reverts #<!-- -->108674 while adding a testcase that triggers a crash in clang-tidy. Fixes #<!-- -->108963. --- Full diff: https://github.com/llvm/llvm-project/pull/109145.diff 2 Files Affected: - (added) clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp (+23) - (modified) clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h (+13-4) ``````````diff 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(); } `````````` </details> https://github.com/llvm/llvm-project/pull/109145 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
