[llvm-commits] TableGen: De-nest if's and fix revealed bug.

[Sean Silva]

I was reading through TableGen's Record.cpp and found a nasty overly nested
`if` situation in resolveTypes. It looks like this:

if (!T1->typeIsConvertibleTo(T2)) {
  if (!T2->typeIsConvertibleTo(T1)) {
    // ...
    // Big long piece of code
    // ...
  }
  return T2;
}
return T1;


Unfortunately, `// Big long piece of code` obscures the fact that these
conversion checks actually are returning the wrong values (or so it seems).
The goal of resolveTypes is to find a common type that both T1 and T2
convert to. De-nested, the code reads:


if (T1->typeIsConvertibleTo(T2))
  return T1;
if (T2->typeIsConvertibleTo(T1))
  return T2;
// ...
// Big long piece of code
// ...


which is obviously wrong, since `T1->typeIsConvertibleTo(T2)` means that T1
converts to T2, and hence T2 should be returned. Remarkably, this mix-up
has no effect on the generated files or on the tests! I tried to come up
with a test that would exercise this, but I couldn't come up with one
because AFAIK all of TableGen's conversions are bidirectional, so
theoretically we could just perform just one of the if's here.

Anyway, I've attached a patch which un-nests the if's and makes the
conversion direction correct (it retains both if's for robustness). There's
some ugly duplicated code nearby that I'll take down once I grok it.

--Sean Silva
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120915/0cfa9fba/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: de-nest-ifs.patch
Type: application/octet-stream
Size: 3482 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120915/0cfa9fba/attachment.obj>