new warnings

/Volumes/SandBox/clang/clang/clang/unittests/VMCore/MetadataTest.cpp: In member function 'virtual void<unnamed>::MDNodeTest_Everything_Test::TestBody()':
/Volumes/SandBox/clang/clang/clang/unittests/VMCore/MetadataTest.cpp:76: warning: dereferencing type-punned pointer will break strict-aliasing rules

:frowning:

Mike,

Can you please just respond to the specific patch on llvm-commits instead of emailing llvm-dev?

Thanks,
-Tanya

Don't happen to know which checkin caused it...

In that case email to llvm-commits list without a reference check-in OR email to llvmbugs list. A patch to fix these warning would be more appropriate for llvmdev list :slight_smile:

Mike Stump wrote:

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

Yes. Constant* is not the same type as MDNode*, ergo loading from an MDNode** as if it were a Constant** is an aliasing violation. It's also just a bad idea for several practical reasons:
1. C++ subtyping is coercive, i.e. it's not necessarily true that converting an MDNode* to a Constant* is a no-op. It probably is in this case, of course, or the code just wouldn't work.
2. Even if the coercion is a no-op, that cast can easily break the type system if you, say, assign an arbitrary Constant* to the Constant**.

John.

Jeffrey Yasskin wrote:

Mike Stump 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. :slight_smile:

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

Thanks. I knew about the 'Derived** is incompatible with Base**' rule for containers and why you can't just downcast to pass to a callee (it might try to insert a Derived2*) but for no good reason thought that different rules applied to type aliasing.

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.

Yes, and in this case neither dynamic_cast nor static_cast would compile.

Sadly, I actually did think about it and wrongly conclude that what I was doing was okay. Apparently I daydream new paragraphs in the C++ standard now. :wink:

Nick