[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+    if (const UnaryOperator *U = dyn_cast<UnaryOperator>(StoreE)) {
+      str = "The expression of the unary operator is an uninitialized value. "
+            "The computed value will also be garbage";
----------------
lebedev.ri wrote:
> xazax.hun wrote:
> > lebedev.ri wrote:
> > > dcoughlin wrote:
> > > > "Unary operator" is probably not something we should expect users to know about. How about just "The expression is an uninitialized value. The computed value will also be garbage."
> > > > 
> > > Yep, i did not like my wording either :)
> > A value being undefined does nt mean uninitialized. E.g.: the result of a bad shift operation might be UndefVal as well.
> > Aren't these diagnostics misleading in those cases? Or we do not care about those?
> I think this is for @dcoughlin to answer.
> But shift operation is a binary operator, so this diff should not change that.
I was wondering about examples like:
```
int x = 1 << -1;
++x;
```

In this particular case, it will not issue the misleading error message. The shift that results in an undefined value will generate an error node, so we will not warn for the pre increment.

But in general I am a bit uncomfortable about the assumption of this check: if a value is undefined the reason is that it is being uninitialized. 

Note that, of course this problem is not specific to this patch but a general question about the checker. 


Repository:
  rC Clang

https://reviews.llvm.org/D40463