[PATCH] D126534: [analyzer] Deadstore static analysis: Fix false positive on C++17 assignments

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Oh nice, I think I've heard about such problems, thanks a lot.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:106
   while (Ex) {
-    const BinaryOperator *BO =
-      dyn_cast<BinaryOperator>(Ex->IgnoreParenCasts());
+    Ex = Ex->IgnoreParenCasts();
+    const BinaryOperator *BO = dyn_cast<BinaryOperator>(Ex);
----------------
It looks like this introduces a change in behavior in unrelated situations, eg. the ones in `objc-arc.m.plist` where the diagnostic is moved to the right:
```lang=c
id obj1 = (__bridge_transfer id)CFCreateSomething(); // expected-warning{{never read}}
          ^before               ^after
```
I think this change is undesirable because it distracts from the assignment. In this case you can easily avoid this problem by using `IgnoreParenImpCasts()` instead, but it also generally makes sense to have patches that target specific situations to match on these situations more precisely. Eg., could you see if you can add a check that a constructor is involved, and maybe check for the exact cast kind that we want to unwrap?


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

https://reviews.llvm.org/D126534