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

[Argyrios Kyrtzidis]

On Jul 18, 2009, at 1:35 PM, Sebastian Redl wrote:

> 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.

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.

-Argiris