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

[Chandler Carruth]

On Wed, Dec 19, 2012 at 2:00 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:

>
> On Dec 19, 2012, at 1:40 PM, Chandler Carruth <chandlerc at google.com>
> wrote:
>
> On Wed, Dec 19, 2012 at 1:37 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:
>
>>
>> On Dec 19, 2012, at 1: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. =]
>>
>>
>> What are you proposing?
>>
>
> Surely the difference in control flow is observable somehow? If this is
> somehow latent and doesn't actually surface, cool, it just seemed strange
> to not be able to make this code misbehave visibly.
>
>
> If you are concerned about this, you should direct some energy towards the
> MI serialization project. It would be very welcome.
>

I didn't know that testing this was blocked on that, sorry. =/ Wasn't
trying to be demanding, it just wasn't clear that there was a particular
barrier to adding a test case.

I'd actually love to spend time here (as would you and others I suspect)
but I too have been unable to prioritize it. I actually have some hope that
in the next year folks here will end up prioritizing it if others don't get
to it sooner. I agree it's an important missing piece
infrastructure-wise...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121219/4271fa4e/attachment.html>