[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.h:27
+// Matches the next statement within the parent statement sequence.
+AST_MATCHER_P(Stmt, hasSuccessor, ast_matchers::internal::Matcher<Stmt>,
+              InnerMatcher) {
----------------
Looking at how this matcher is used, I'm starting to doubt this is an efficient way to find the kind of the coding pattern this check targets. Creating CFG for each declaration statement would result in a lot of overlapping and, thus, unnecessary work. I guess, it should be easy to create a test case (e.g. thousands of declarations in the same function that would trigger one or better both hasSuccessor matchers) where this check would run for extremely long time. I'd recommend doing this to test this or any alternative solution you come up with. Performance problems are very real for some clang-tidy checks. And this check raises a number of red flags w.r.t. performance.

An alternative I could suggest is to build CFG for each function once and then search for the interesting declarations while traversing it (instead of running AST matchers and building CFG inside them).


https://reviews.llvm.org/D37014