[cfe-dev] Microsoft vtordisp field support for clang.

[r4start]

On 15/02/2012 23:28, John McCall wrote:
> On Feb 15, 2012, at 4:37 AM, r4start wrote:
>> Please see this patch.
> You don't need the store both the set of vbases with vtordisps and the
> additional information in the virtual-base-offset map.  I would prefer that
> you rely only on the second;  it has the distinct virtual of imposing no
> extra overload to the layouts of classes without virtual bases.
>
>> I am not sure that new algorithm very efficient, but I think new algorithm is better than old one.
> Why is this influenced by whether the class has a new virtual function?
This check need if we have something like this:
class first{
public:
   virtual void asdf() {}
     virtual void g(){}
};
class second : public virtual first {
public :
   second(){}
   virtual void g(){}
   virtual void asdf() {}
};
class third : public virtual second {
public:
   third(){}
   virtual void g() {}
   virtual void asdf() {}
};
Only first will have vtordisp. Also flags in condition help to ignore 
this check in next case:
struct AV {
   virtual void foo();
};
struct BV : AV {
};
struct C : virtual BV {
   C();
   virtual void foo();
};
> Here's my suggested (untested) implementation:
>
> void RecordLayoutBuilder::getRequiredVtorDisps(const CXXRecordDecl *RD, llvm::SmallPtrSet<const CXXRecordDecl*>  &VBasesRequiringVtorDisp) {
>    if (RD->vbases_empty()) return;
>
>    // As an optimization, remember the set of virtual bases that we haven't decided yet.
>    llvm::SmallPtrSet<const CXXRecordDecl *>  UndecidedVBases;
>    for (CXXRecordDecl::base_class_const_iterator I = RD->vbases_begin(),
>         E = RD->vbases_end(); I != E; ++I) {
>      UndecidedVBases.insert(*I);
>    }
>
>    // A virtual base requires a vtor disp if any of our base classes require it to have one.
>    for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
>         E = RD->bases_end(); I != E; ++I) {
>      const CXXRecordDecl *BaseDecl = I->getType()->getAsCXXRecordDecl();
>      const ASTRecordLayout&BaseLayout = Context.getASTRecordLayout(BaseDecl);
>      for (ASTRecordLayout::VBaseOffsetsMapTy::const_iterator
>            VI = BaseLayout.VBaseOffsetsMap.begin(), VE = BaseLayout.VBaseOffsetsMap.end(); VI != VE; ++VI) {
>        if (!VI->second.IsVtorDispPresent) continue;
>        if (!UndecidedVBases.erase(VI->first)) continue;
>        VBasesRequiringVtorDisp.insert(VI->first);
>        if (UndecidedVBases.empty()) return;
>      }
>    }
>
>    // Otherwise, a virtual base requires a vtor disp if any the non-destructor
>    // virtual methods declared in this class directly override a method provided
>    // by that virtual base.
>
>    // Collect the set of bases directly overridden by any method in this class.
>    // It's possible that some of these classes won't be virtual bases, or won't be
>    // provided by virtual bases, or won't be virtual bases in the overridden
>    // instance but are virtual bases elsewhere.  Only the last matters for what
>    // we're doing, and we can ignore those:  if we don't directly override
>    // a method provided by a virtual copy of a base class, but we do directly
>    // override a method provided by a non-virtual copy of that base class,
>    // then we must indirectly override the method provided by the virtual base,
>    // and so we should already have collected it in the loop above.
>    llvm::SmallPtrSet<const CXXRecordDecl*>  OverriddenBases;
>    for (CXXRecordDecl::method_iterator M = RD->method_begin(),
>         E = RD->method_end(); M != E; ++M) {
>      if (!M->isVirtual() || isa<CXXDestructorDecl>(M)) continue;
>      for (CXXMethodDecl::method_iterator I = M->begin_overridden_methods(),
>            E = M->end_overridden_methods(); I != E; ++I) {
>        const CXXRecordDecl *OverriddenBase = (*I)->getParent();
>
>        // As an optimization, check immediately whether we're overriding
>        // something from the undecided set.
>        if (UndecidedVBases.erase(OverriddenBase)) {
>          VBasesRequiringVtorDisp.insert(OverriddenBase);
>          if (UndecidedVBases.empty()) return;
>          continue;
>        }
>
>        // Otherwise, just collect it.
>        OverriddenBases.insert(OverriddenBase);
>      }
>    }
>
>    // Walk the undecided v-bases and check whether they (non-virtually)
>    // provide any of the overridden bases.
>    for (llvm::SmallPtrSet<const CXXRecordDecl*>::const_iterator
>          I = UndecidedVBases.begin(), E = UndecidedVBases.end(); I != E; ++I) {
>      if (hasAnyNonVirtualBase(*I, OverriddenBases))
>        VBasesRequiringVtorDisp.insert(*I);
>    }
> }
>
> Where hasAnyNonVirtualBase just recursively walks the non-virtual
> class hierarchy and checks whether any of the bases are in the set.
>
Thx for your algorithm, it works fine in general. I add one condition 
and write hasAnyNonVirtualBase.
If realization is ok, then I cleanup code and relocate it to ToT.
Also I include patch for condition in Layout function, this patch must 
be committed before vtordisp patch.

  - Dmitry.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ms-layout.patch
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120216/fb9980bb/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: new-vtordisp.patch
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120216/fb9980bb/attachment-0001.ksh>