rsmith added inline comments. ================ Comment at: lib/CodeGen/CGClass.cpp:1832 @@ +1831,3 @@ + // Generate vtable assumptions if we are calling dynamic class ctor + // and we are not in another ctor. + if (CGM.getCodeGenOpts().OptimizationLevel > 0 && ---------------- Prazek wrote: > rsmith wrote: > > Prazek wrote: > > > hfinkel wrote: > > > > I think this comment should be a little more verbose. How about this: > > > > > > > > // Generate vtable assumptions if we are calling dynamic-class's > > > > ctor, except when doing so as part of a derived class's ctor's > > > > base-class initialization. Doing so in this latter case would be > > > > useless, because the vtable is about to be overwritten by the derived > > > > class's vtable. > > > > > > > The main point of not calling this function, is because it is useless to > > > have assumption loads inside constructor, when they are generated also > > > outside of ctor. I guess You could have case when You are not overriding > > > base vptr, and You are calling base ctor from dynamic class > > It might also be worth including in the comment that it is not correct to > > call `EmitVTableAssumptionLoads` here, because it assumes the object's vptr > > points to the complete object vtable; during a constructor call it will > > probably have a construction vtable instead. > Well, it would be correct, but it would be useless. Just after calling base > constructor, assume loads would appear, and after there would be new store to > vptr. `EmitVTableAssumptionLoads` assumes that the `This` pointer points to an object whose most-derived type is `ClassDecl`, and it assumes that the object is not under construction. Both of those assumptions can be violated for a base class constructor call, and can result in it emitting incorrect assumptions (because virtual bases may be at different offsets and because the vptr of an object under construction may be different from the corresponding vptr of a fully-constructed object).
http://reviews.llvm.org/D11859 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
