Author: David Meng Date: 2026-08-21T12:02:39+03:00 New Revision: 8d9145680dbc6e56c801d324745043b098741f8d
URL: https://github.com/llvm/llvm-project/commit/8d9145680dbc6e56c801d324745043b098741f8d DIFF: https://github.com/llvm/llvm-project/commit/8d9145680dbc6e56c801d324745043b098741f8d.diff LOG: [clang-tidy] Fix invalid suggestion for a macro container in `performance-inefficient-algorithm` (#217713) `on()` matches the implicit object argument after `IgnoreParenImpCasts()`, so for `#define SET_RANGE s.begin(), s.end()` the check bound the reference to `s` inside the macro body. That range covers only part of the macro expansion, so `Lexer::getSourceText` returned an empty string and the suggested fix came out as `.find(46)`. Emit no fix when there is no source text for the container, as is already done for the searched-for value. Fixes https://github.com/llvm/llvm-project/issues/217711 Related https://github.com/llvm/llvm-project/pull/217458 Added: Modified: clang-tools-extra/clang-tidy/performance/InefficientAlgorithmCheck.cpp clang-tools-extra/docs/ReleaseNotes.md clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-algorithm.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/performance/InefficientAlgorithmCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientAlgorithmCheck.cpp index c5e66db6f2d89..aa3c6fa5c1dab 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientAlgorithmCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/InefficientAlgorithmCheck.cpp @@ -134,8 +134,8 @@ void InefficientAlgorithmCheck::check(const MatchFinder::MatchResult &Result) { SM, LangOpts); // There is no source text for an expression that covers only part of a // macro expansion. Building the replacement from an empty string would - // incorrectly drop the value. - if (!ParamText.empty()) { + // incorrectly drop the container or the value. + if (!ContainerText.empty() && !ParamText.empty()) { const std::string ReplacementText = (llvm::Twine(ContainerText) + (PtrToContainer ? "->" : ".") + AlgDecl->getName() + "(" + ParamText + ")") diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 9cead803ad0e5..843ef70c0452c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -155,11 +155,12 @@ infrastructure are described first, followed by tool-specific sections. diff erent constructor. - Improved {doc}`performance-inefficient-algorithm - <clang-tidy/checks/performance/inefficient-algorithm>` check by copying the - searched-for value as written rather than stripping its parentheses, which - could produce an invalid fix such as `s.find()` when the value came from a - macro. No fix is offered when the value covers only part of a macro - expansion. + <clang-tidy/checks/performance/inefficient-algorithm>` check to no longer + produce a fix with the container or the searched-for value missing, such as + `.find(43)` or `s.find()`, when either comes from a macro. The value is + copied as written rather than with its parentheses stripped, and no fix is + offered when an argument covers only part of a macro expansion, as it then + has no source text of its own. - Improved {doc}`readability-enum-initial-value <clang-tidy/checks/readability/enum-initial-value>` check by adding diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-algorithm.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-algorithm.cpp index 0757ecb603d79..c4f685f35b13c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-algorithm.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-algorithm.cpp @@ -86,6 +86,9 @@ int main() { auto p = std::find(s.begin(), s.end(), (43)); // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be // CHECK-FIXES: auto p = s.find((43)); + auto r = std::find((s).begin(), (s).end(), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be + // CHECK-FIXES: auto r = s.find(43); int i = 1, j = 2; auto q = std::find(s.begin(), s.end(), (i++, j)); // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be @@ -113,6 +116,10 @@ int main() { // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be // CHECK-FIXES: msptr->find(46); + find((msptr)->begin(), (msptr)->end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: msptr->find(46); + it = std::find(s.begin(), s.end(), 43, std::greater<int>()); // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: diff erent comparers used in the algorithm and the container [performance-inefficient-algorithm] @@ -202,3 +209,60 @@ void macroExpansion(std::set<int> s, int i) { // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be // CHECK-FIXES: find(s.begin(), VAL_AND_END); } + +#define PAREN_CONT (s) +#define PLAIN_CONT s +#define RANGE s.begin(), s.end() +#define PTR_RANGE p->begin(), p->end() +#define CONT_AND_BEGIN s.begin() +#define BARE_RANGE_OF(c) c.begin(), c.end() +#define PAREN_RANGE_OF(c) (c).begin(), (c).end() +#define MY_SET s +#define NESTED_RANGE MY_SET.begin(), MY_SET.end() + +// The container text comes from the reference to it, with any parentheses +// around it stripped. A reference spelled where the macro was expanded, such as +// a macro argument, is read from there, and one that spans a whole expansion +// keeps it intact. A reference covering only part of an expansion has no source +// text of its own, and the call is then diagnosed without a fix. +void macroContainer(std::set<int> s, std::set<int> *p) { + find(BARE_RANGE_OF(s), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: s.find(43); + + find(BARE_RANGE_OF(MY_SET), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: MY_SET.find(43); + + find(PLAIN_CONT.begin(), PLAIN_CONT.end(), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: PLAIN_CONT.find(43); + + find(RANGE, 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: find(RANGE, 43); + + count(PTR_RANGE, 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: count(PTR_RANGE, 43); + + find(CONT_AND_BEGIN, s.end(), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: find(CONT_AND_BEGIN, s.end(), 43); + + find(NESTED_RANGE, 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: find(NESTED_RANGE, 43); + + find(PAREN_RANGE_OF(s), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: s.find(43); + + find(PAREN_CONT.begin(), PAREN_CONT.end(), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: find(PAREN_CONT.begin(), PAREN_CONT.end(), 43); + + find(RANGE, PLAIN_VALUE); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: find(RANGE, PLAIN_VALUE); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
