[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

I really-really like this change. The results look great, though I'm not sure what happened here <http://cc.elte.hu:15200/Default/#is-unique=on&run=before%20deadstore%20option&newcheck=after%20deadstore%20option&review-status=Unreviewed&review-status=Confirmed&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=before%20deadstore%20option_diff_after%20deadstore%20option&reportHash=add89d78259f010f6f650d924b429e0e&report=72&subtab=add89d78259f010f6f650d924b429e0e>, are we sure that this originates from the analyzer, and is not some storing error?

In any case, please use `clang-format-diff.py` on this patch, otherwise LGTM.



================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:644
+                  "E.g.: if ((P = f())) where P is unused."
+                  "It will likely report false positives.",
+                  "false",
----------------
Mmm, how about we soften this warning? :)

`"Enabling this may result in some false positive finds.`


================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:646
+                  "false",
+                  InAlpha>
+  ]>,
----------------
We mark checker options that are still in development with `InAlpha`, but I feel like this feature is as good as it gets. Feel free to mark it as `Released`, so it'll be user facing!


================
Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:223
 
-      case Enclosing:
-        // Don't report issues in this case, e.g.: "if (x = foo())",
-        // where 'x' is unused later.  We have yet to see a case where
-        // this is a real bug.
-        return;
+      case Enclosing: // eg.:f((x = foo()))
+        if (!WarnForDeadNestedAssignments)
----------------
Could you please move this comment to the previous line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66733