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

[Chris Lattner]

On Jul 18, 2009, at 2:05 PM, Argyrios Kyrtzidis wrote:
>>
>> 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.
>
> How about I make the inheritance private ? The interface will have  
> to be duplicated to FunctionDecl and VarDecl
> (and any other class that may use Redeclarable) but at least it will  
> only delegate to the implementation of Redeclarable.

As Sebastian points out, our use isa/dyncast etc and other tricks for  
efficient memory use sometimes run afoul of multiple inheritance.  I  
generally try to avoid it where possible in any case, except where  
there is no other good option (e.g. around DeclContext, which I still  
don't think is a good idea ;-).  Private inheritance doesn't help  
either.

Is there any reason not to introduce a new class that these things  
directly inherit from?

-Chris