[cfe-commits] r76297 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/AST/Redeclarable.h lib/AST/Decl.cpp

[Sebastian Redl]

Argyrios Kyrtzidis wrote:
> On Jul 18, 2009, at 11:40 AM, Chris Lattner wrote:
>
>   
>> This is nice Argiris, but is there a way to do with with containment
>> instead of multiple inheritance?
>>     
>
> I don't quite see the problem in this case, it only adds a  
> PointerIntPair and a public interface to the derived subclass that  
> will get used through
> the subclass (e.g. through FunctionDecl), Redeclarable is not actually  
> supposed to get used anywhere else by client code and the rest of the  
> code
> is not supposed to be aware of it (you can cast to it but what's the  
> point).
> Can you elaborate on what do you think the problem is ?
>   

I don't know what Chris's objection is, but I believe that various
things we do in the Clang codebase would be very fragile in the face of
multiple inheritance. Most importantly, in passing things back and forth
between Sema and Parse, we sometimes cast derived classes to void* and
then cast the void* to a base pointer. This works as long as the base is
in the "primary" inheritance chain, i.e. every class in the chain is the
first in the list of bases. If that isn't the case, the base and derived
pointers do not have the same value, and the void cast will mess it up.
Your patch isn't likely to invoke this particular problem, but it is a
very subtle one, and I believe that it is too much to ask of the Clang
developers to always remember this caveat. It's easier to avoid MI in
the first place.

Sebastian