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

Yes but these issues apply only if you are going to use and pass  
around Redeclarable<FunctionDecl>* pointers,
which is not the case here. Redeclarable will not appear anywhere else.

-Argiris