dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.
Thanks, this looks good to me!
Do you have commit access or do you need someone to commit it for you?
================
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";
----------------
xazax.hun wrote:
> 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.
It is a good point, but I think fixing it should wait until a later patch.
Repository:
rC Clang
https://reviews.llvm.org/D40463
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
