This is an archive of the discontinued LLVM Phabricator instance.

[mips] Eliminate instances of "potentially uninitialised local variable" warnings, NFC
ClosedPublic

Authored by s.egerton on Mar 11 2016, 7:37 AM.

Details

Summary

This should eliminate all occurrences of this within LLVMMipsAsmParser.
This patch is in response to http://reviews.llvm.org/D17983. I was unable
to reproduce the warnings on my machine so please advise if this fixes the
warnings.

Diff Detail

Event Timeline

s.egerton updated this revision to Diff 50431.Mar 11 2016, 7:37 AM
s.egerton retitled this revision from to [mips] Eliminate instances of "potentially uninitialised local variable" warnings, NFC.
s.egerton updated this object.
s.egerton added a subscriber: llvm-commits.

I'm still not sure cleaning up these warnings is worthwhile. If we aren't
going to get warning clean & keep the warning on/maintained (in which case
I'd really advocate for us to use Clang's version and/or implement the
necessary variant in Clang) what's the merit in rephrasing the cases that
aren't buggy (this appears to be such a case - correct me if I'm wrong?

ariccio edited edge metadata.Mar 11 2016, 1:20 PM

I'm still not sure cleaning up these warnings is worthwhile. If we aren't
going to get warning clean & keep the warning on/maintained (in which case
I'd really advocate for us to use Clang's version and/or implement the
necessary variant in Clang) what's the merit in rephrasing the cases that
aren't buggy (this appears to be such a case - correct me if I'm wrong?

Generally, I agree, but the resulting code here is quite a bit clearer? Perhaps if not for the sake of cleaning up warnings, there are just fewer codepaths to maintain here?

vkalintiris accepted this revision.Mar 14 2016, 3:37 AM
vkalintiris edited edge metadata.

LGTM. Thanks for working on this.

This revision is now accepted and ready to land.Mar 14 2016, 3:37 AM
dsanders accepted this revision.Mar 14 2016, 6:40 AM
dsanders edited edge metadata.

I'm still not sure cleaning up these warnings is worthwhile. If we aren't
going to get warning clean & keep the warning on/maintained (in which case
I'd really advocate for us to use Clang's version and/or implement the
necessary variant in Clang) what's the merit in rephrasing the cases that
aren't buggy (this appears to be such a case - correct me if I'm wrong?

Generally, I agree, but the resulting code here is quite a bit clearer? Perhaps if not for the sake of cleaning up warnings, there are just fewer codepaths to maintain here?

This is my thinking too, the warning was a false-positive but the previous code was also unnecessarily complicated.

s.egerton closed this revision.Mar 17 2016, 3:42 AM