[LLVMdev] new warnings

[Jeffrey Yasskin]

On Tue, Apr 7, 2009 at 8:31 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> Mike Stump wrote:
>> On Apr 7, 2009, at 6:02 PM, Tanya M. Lattner wrote:
>>> Can you please just respond to the specific patch on llvm-commits
>>> instead
>>> of emailing llvm-dev?
>>
>> Don't happen to know which checkin caused it...
>
> Given that there's only ever been one checkin to this file, it can't be
> that hard. :)
>
> But I'm not sure what it's complaining about. MDNode isa Constant, 'n1'
> isa MDNode*, hence &n1 isa MDNode**. Casting that to Constant** breaks
> strict-aliasing rules? Really?

Yes, because MDNode* is a different type from Constant*. In the C++0X
draft, it's in section 3.10:

If a program attempts to access the stored value of an object through
an lvalue of other than one of the
following types the behavior is undefined

— the dynamic type of the object,
— a cv-qualified version of the dynamic type of the object,
— a type similar (as defined in 4.4) to the dynamic type of the object,
(This describes const/volatile qualification)
— a type that is the signed or unsigned type corresponding to the
dynamic type of the object,
— a type that is the signed or unsigned type corresponding to a
cv-qualified version of the dynamic type
of the object,
— an aggregate or union type that includes one of the aforementioned
types among its members (including,
recursively, a member of a subaggregate or contained union),
— a type that is a (possibly cv-qualified) base class type of the
dynamic type of the object,
— a char or unsigned char type.

Constant* is not a base class type of MDNode* even though Constant is
a base class of MDNode. The conversion would be unsafe even without
strict aliasing if you weren't actually going to Constant*const*.

You should instead write:

Constant *const tmp = n1;
MDNode *n2 = MDNode::get(&tmp, 1);

I'm not sure about LLVM's conventions, but I think it's a good idea to
avoid C-style cases in favor of the more specific (but longer) C++
ones. Then the compiler will tell you about dangerous casts by making
you use reinterpret_cast or const_cast to do them. And
reinterpret_cast is only safe when going to (or sometimes from) char*
or intptr_t.

Jeffrey