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

[Dmitri Gribenko]

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.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/