[clang] [analyzer] Ignoring `T v=v; ` idiom for uninitialized variable checker and dead store checker (PR #187530)

[Aaron Ballman via cfe-commits]

================
@@ -0,0 +1,65 @@
+// RUN: %clang_analyze_cc1 %s -verify -xc \
+// RUN:   -analyzer-checker=core,debug.ExprInspection,deadcode.DeadStores
+
+// Suppress the C++ -Wuninitialized warnings on struct and reference.
+// RUN: %clang_analyze_cc1 %s -verify -xc++ -Wno-uninitialized \
+// RUN:   -analyzer-checker=core,debug.ExprInspection,deadcode.DeadStores
+
+// Self assignment initialization in C code will be treated as nop.
+// We will report the VarDecl only if it was left uninitialized by the time of
+// a subsequent DeclRefExpr.
+
+// NOTE: No warnings from the deadcode.DeadStores checker.
+
+void clang_analyzer_warnIfReached();
+
+struct S { int x; };
+union U { int x; };
+enum T { TT };
+
+// No need to test VarDecl of multiple variables, as they will be split into
+// single ones when constructing the CFG.
+
+int warnvar() {
+  int x = x; // no-warnings for C/C++, binding is skipped via the
----------------
AaronBallman wrote:

> > > The reasons are similar to the T v = (v); suggestion above. Therefore, I am open to int x{x}; and int x(x);, although I don't think C++ developers will suppress an unused-var warning in this way.
> > 
> > 
> > Oh that's right, this is suppressed for C code rather than C++. Huh, I suppose the question there is whether we handle `int c = { c };`
> 
> `int c (c);` has the same AST structure as `int c = c;`, so it is currently handled. 
> `int c {c};` is different, so perhaps we can also add this, as `int c (c);` has already been handled.
> `int c = {c};` has the same AST structure as `int c {c};`.

Hmm, it probably makes sense to handle `int c{c};` then as well, but if that turns out to be a slog, I don't insist.

> > I'm aware -- the goal is to ensure we don't regress the behavior in the future (we don't ever want a self-init warn on this case despite it looking somewhat self-init-like at first glance).
> 
> OK. But I actually don't get the point of this test. We are checking the AST rather than the literal text of the code. There seems to be no chance that we get them mixed up.

Agreed that I don't expect we'd ever get them mixed up, and this isn't something I'd block the PR over. However, having done this for long enough, "there's no chance" tends to turn into "oops, there was a chance" if there's not test coverage ensuring we catch regressions (e.g., a whole lot of "that can never happen" issues started happening when we added clang-repl because that handles partial program constructs which we never considered before). So I tend to ask for more tests even if they're sometimes a bit silly; ideally we'd have an up-to-date coverage report so we could have more confidence and maybe add fewer tests, but that's a bigger lift.

https://github.com/llvm/llvm-project/pull/187530