[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 7 14:20:03 PST 2018

JonasToth added a comment.

I agree with @Eugene.Zelenko that this check should rather live in `bugprone-` as the issue its diagnosing is not LLVM specific. It is ok to add an alias into the llvm module.

Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:22
+void ProblematicStaticsCheck::registerMatchers(MatchFinder *Finder) {
+  auto Matcher = returnStmt(hasReturnValue(ignoringImplicit(unaryOperator(
+                                hasOperatorName("&"),
Nit: you can inline this variable, having it does not improve readability (IMHO) nor does it reduce duplication.

Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:26
+                                    varDecl(isStaticLocal()).bind("var"))))))),
+                            isExpansionInFileMatching("/include/.*\\.h"))
+                     .bind("return");
I think the matching on `include` should not be done. Maybe it's ok to filter for header files, but we should not assume they live in `include/...` (as i think this check is generally useful and not too LLVM specific).

More filters for false positives could be checking if the function itself is `static` or in an anonymous namespace.

Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:33
+  const auto *VD = Result.Nodes.getNodeAs<VarDecl>("var");
+  const auto *Return = Result.Nodes.getNodeAs<ReturnStmt>("return");
+  diag(Return->getBeginLoc(), "address of static local variable %0 may not "
Please `assert` on `VD` and `Return`

Comment at: test/clang-tidy/llvm-problematic-statics.cpp:3
+#include "include/ProblematicStatics.h"
+// CHECK-MESSAGES: ProblematicStatics.h:7:5: warning: address of static local variable 'index' may not be identical across library boundaries [llvm-problematic-statics]
I think the testing should include more cases. At least normal functions, and a false case as well.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list