Page MenuHomePhabricator

Make -Wuninitialized warn on pointer-to-member and comma operators.
ClosedPublic

Authored by epertoso on Dec 11 2013, 9:32 AM.

Details

Reviewers
klimek
rsmith
Summary

A few lines taken from in http://llvm-reviews.chandlerc.com/D2181 that are unrelated to the main goal of that patch.

isTrackedVar has been updated to also track records.
DeclRefExprs appearing on the left side of a comma operator are ignored, while those appearing on the right side are classified as Use.

Diff Detail

Event Timeline

Does that comma change do the right thing for "(foo(a), b)"? (i.e. does it still warn about 'a' being uninitialized?)

epertoso updated this revision to Unknown Object (????).Dec 12 2013, 6:05 AM

Yes, it does.

DeclRefExprs are visited only by TransferFunctions, while ClassifyRefs relies mostly on binary operators, casts and unary operators to determine whether a variable is used or initialized.

For statements like c = (a, a) we need to classify the DeclRefExpr on the LHS of the comma operator as Ignore, or TransferFunctions will treat is as an initialization.

In the case of (foo(a), b), the LHS is a CallExpr and ClassifyRefs::classify has no effect on it, while a is the sub-expression of an lvalue-to-rvalue cast, and will be correctly classified as Use.

I added two lines to the test.

rsmith added inline comments.Dec 12 2013, 3:14 PM
lib/Analysis/UninitializedValues.cpp
436–457

It doesn't look like there's a new test case for this change. Is this somehow covered by our existing tests?

epertoso updated this revision to Unknown Object (????).Dec 13 2013, 10:08 AM
epertoso added inline comments.
lib/Analysis/UninitializedValues.cpp
436–457

Added one.

rsmith accepted this revision.Dec 13 2013, 11:32 AM

LGTM

Hi Richard,

even if phabricator mixes me up with the svn user 'enrico', I cannot commit.

epertoso updated this revision to Diff 20300.Feb 19 2015, 7:27 AM
epertoso edited edge metadata.
epertoso removed rL LLVM as the repository for this revision.

Updated to diff against head, as it should have been submitted quite a while ago.

Richard, would you mind having another look at this?
It changed slightly after your LGTM.

klimek accepted this revision.Mar 3 2015, 6:20 AM
klimek added a reviewer: klimek.

LG. I think this looks fine from the version richard lg'ed.

klimek closed this revision.Jul 3 2015, 7:04 AM