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

[Jakob Stoklund Olesen]

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.

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121219/67d6b0ae/attachment.html>