[llvm] r178147 - Cleanup the simplify_type implementation.

Eli Friedman eli.friedman at gmail.com
Mon Jul 15 23:23:10 PDT 2013


On Mon, Jul 15, 2013 at 10:11 PM, Rafael EspĂ­ndola
<rafael.espindola at gmail.com> wrote:
>> The extra enable_ifs were intentionally added in r175819 to avoid the
>> accidental usage of unsafe casts.  Is there some justification for why
>> they are no longer necessary?
>>
>> (Sorry about digging up old commits... I just happened to notice this now.)
>
> NP. Sorry I missed that. I think the attached patch fixes it, but do
> you have any suggestions on how to prevent this being lost again? Do
> we have some form of negative compilation test or should I try to use
> sfinae in a test to check that we cannot cast an rvalue?

I don't think we have negative compilation tests... the regression
tests don't use the host compiler or include LLVM headers, and the
unit-tests are all supposed to compile.

In terms of testing it in the unittests, though, you shouldn't need
any fancy SFINAE; just do something like:

using namespace llvm;
struct IllegalCast;
template<typename T> IllegalCast *cast(...) { return 0; }
IllegalCast *testIllegalCast() {
  return cast<SomeType>(SomeRValue);
}

This will compile if and only if the cast isn't viable, because
overload resolution will always prefer a function with named
arguments.

In terms of the patch itself, if I recall correctly, the enable_ifs
you're removing are necessary to reject a T* where T is simplifiable.
I could be wrong, though.

-Eli




More information about the llvm-commits mailing list