[PATCH] D118657: [analyzer] Adds TrustReturnsNonnullChecker

Artem Dergachev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 1 12:21:21 PST 2022


NoQ added a comment.

Great, thanks!

The phabricator expects complete patches reuploaded (i.e., diffs against main branch rather than diffs against the previous upload); but it knows how to produce patches-on-patches from complete patches through the History tab. Another reason to upload complete patches is to unconfuse pre-merge buildbots (the "Build Status: patch application failed" part).

So please re-upload and that'd be a ✅



================
Comment at: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt:114-115
   TraversalChecker.cpp
   TrustNonnullChecker.cpp
+  TrustReturnsNonnullChecker.cpp
   UndefBranchChecker.cpp
----------------
As this checker mimics that other checker, the question inevitably arises whether the two can be merged. The other checker is currently stuffed with extra code to handle operator `==` which is possibly no longer necessary since D82445 provides a more general solution. I guess this could be an interesting refactoring pass.


================
Comment at: clang/test/Analysis/test-decl-crash.cpp:6
+void test(void *(*f)(void)) {
+  // will probably crash the compiler, worth a test case
+  f();
----------------
It makes sense to put the new test into the same file given that they share the `RUN:` line.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118657/new/

https://reviews.llvm.org/D118657



More information about the llvm-commits mailing list