[cfe-commits] r83426 - in /cfe/trunk: lib/CodeGen/CGCXX.cpp lib/CodeGen/CGCXXClass.cpp lib/CodeGen/CodeGenFunction.h test/CodeGenCXX/virtual-base-cast.cpp

[Douglas Gregor]

On Oct 6, 2009, at 3:43 PM, Anders Carlsson wrote:

> Author: andersca
> Date: Tue Oct  6 17:43:30 2009
> New Revision: 83426
>
> URL: http://llvm.org/viewvc/llvm-project?rev=83426&view=rev
> Log:
> Change GetAddressCXXOfBaseClass to use CXXBasePaths for calculating  
> base class offsets. Fix the code to handle virtual bases as well.

Excellent!

>
> -          const CXXMethodDecl *MD = *mi;
> +           ++mi) {
> +        if (!mi->isVirtual())
> +          continue;
> +
> +        const CXXMethodDecl *MD = *mi;
> +        llvm::Constant *m = 0;
> +//        if (const CXXDestructorDecl *Dtor =  
> dyn_cast<CXXDestructorDecl>(MD))
> +//          m = wrap(CGM.GetAddrOfCXXDestructor(Dtor,  
> Dtor_Complete));
> +//        else {

Do we need a FIXME here? Commented-out code makes me nervous :)

>
> +static uint64_t
> +ComputeNonVirtualBaseClassOffset(ASTContext &Context, CXXBasePaths  
> &Paths,
> +                                 unsigned Start) {
> +  uint64_t Offset = 0;
> +
> +  const CXXBasePath &Path = Paths.front();
> +  for (unsigned i = Start, e = Path.size(); i != e; ++i) {
> +    const CXXBasePathElement& Element = Path[i];
>
> +    // Get the layout.
> +    const ASTRecordLayout &Layout = Context.getASTRecordLayout 
> (Element.Class);
> +
> +    const CXXBaseSpecifier *BS = Element.Base;
> +    assert(!BS->isVirtual() && "Should not see virtual bases here!");
> +
> +    const CXXRecordDecl *Base =
> +      cast<CXXRecordDecl>(BS->getType()->getAs<RecordType>()- 
> >getDecl());
>
> -    return Offset;
> +    // Add the offset.
> +    Offset += Layout.getBaseClassOffset(Base) / 8;
> +  }
> +
> +  return Offset;
> }

Very nice!

>
> +static llvm::Value *GetCXXBaseClassOffset(CodeGenFunction &CGF,
> +                                          llvm::Value *BaseValue,
> +                                          const CXXRecordDecl  
> *ClassDecl,
> +                                          const CXXRecordDecl  
> *BaseClassDecl) {
> +  CXXBasePaths Paths(/*FindAmbiguities=*/false,
> +                     /*RecordPaths=*/true, /*DetectVirtual=*/true);
> +  if (!const_cast<CXXRecordDecl *>(ClassDecl)->
> +        isDerivedFrom(const_cast<CXXRecordDecl *>(BaseClassDecl),  
> Paths)) {
> +    assert(false && "Class must be derived from the passed in base  
> class!");
> +    return 0;
> +  }

It would be nice if we could assert that there are no ambiguities  
here, just to make sure we didn't slip up somewhere in Sema. However,  
we don't want to look for ambiguities in -Asserts mode, since that's  
less efficient.

   - Doug