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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to