[llvm-commits] [llvm] r162230 - in /llvm/trunk: lib/CodeGen/MachineVerifier.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp test/CodeGen/ARM/constants.ll

[Eli Bendersky]

On Wed, Dec 19, 2012 at 2:16 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> On Wed, Dec 19, 2012 at 11:10 PM, Chandler Carruth <chandlerc at google.com> wrote:
>> On Wed, Dec 19, 2012 at 1:04 PM, Dmitri Gribenko <gribozavr at gmail.com>
>> wrote:
>>>
>>> On Wed, Dec 19, 2012 at 11:02 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>
>>> wrote:
>>> >
>>> > On Dec 19, 2012, at 11:01 AM, Dmitri Gribenko <gribozavr at gmail.com>
>>> > wrote:
>>> >
>>> >> On Tue, Aug 21, 2012 at 12:39 AM, Jakob Stoklund Olesen
>>> >> <stoklund at 2pi.dk> wrote:
>>> >>>       ++MBBI;
>>> >>>       if (MBBI == MF->end()) {
>>> >>>         report("MBB conditionally falls through out of function!",
>>> >>> MBB);
>>> >>> -      } if (MBB->succ_size() != 2) {
>>> >>> +      } if (MBB->succ_size() == 1) {
>>> >>
>>> >> Hello Jakob,
>>> >>
>>> >> Sorry to comment on an old commit, but '} if (...' looks suspicious --
>>> >> 'else' is probably missing.  I have just fixed two bugs of this kind
>>> >> in Clang, found by grep '} if'.
>>> >
>>> > Yikes, that does indeed look like a missing else.
>>> >
>>> > Please go ahead and fix it.
>>>
>>> OK to commit without a testcase?
>>
>>
>> I'd like a testcase. =]
>>
>> Dmitri, want to try your hand at building a Clang warning for this? I just
>> chatted with Richard and he seemed to think it could work by warning in the
>> Parser when looking for an 'else' token, finding an 'if' token, and the '}'
>> and 'if' tokens' spelling locations are on the same line. (or checking the
>> start-of-line flag on the 'if' token, but Richard thought there might be
>> problems there).
>
> That's a good idea.  So far LLVM/Clang had three cases of this, and
> all are true bugs.
>

I wonder if other open-source projects have this bug somewhere as
well. Maybe some code-search online thingy can help find out. If you
go ahead and add a new Clang warning, it's important to know whether
there are going to be legit uses that will be warned about.

Eli