[PATCH] D38717: Patch to Bugzilla 31373

Eli Friedman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 10 15:48:55 PDT 2017


efriedma added a comment.

Please make sure the title (subject line) for a patch reflects the actual contents of the change, as opposed to referring to Bugzilla; a lot of people skim cfe-commits, so you want to make sure interested reviewers find you patch.

Needs a regression test to verify we generate the warning in cases we should, and don't generate the warning in cases where we shouldn't (probably want a few tests with variables whose type is a class with a non-trivial destructor, or a reference to a temporary with a non-trivial destructor; removing the variable could change the behavior in those cases).  There are plenty of examples to follow in "test/SemaCXX/" in the source.

I don't really like messing with the value of the "Used" bit; isUsed() is supposed to reflect whether a variable is odr-used, and getting it wrong can have weird implications for the way we type-check and emit code.  (Most of those implications don't really apply to local variables, but it's still confusing.)  isReferenced() is our "is this declaration doing anything useful" bit.  Can we somehow change the way we set and use the "Referenced" bit to come up with the right result here?


https://reviews.llvm.org/D38717





More information about the cfe-commits mailing list