<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">On Jul 19, 2013, at 12:29 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br><div><blockquote type="cite"> - Add a big comment block that should have been there and minor stuff.<br></blockquote><div><br></div><div> void CodeGenModule::EmitCXXDestructors(const CXXDestructorDecl *D) {</div><div>- // The destructor in a virtual table is always a 'deleting'</div><div>- // destructor, which calls the complete destructor and then uses the</div><div>- // appropriate operator delete.</div><div>- if (D->isVirtual())</div><div>- EmitGlobal(GlobalDecl(D, Dtor_Deleting));</div><div>-</div><div>- // The destructor used for destructing this as a most-derived class;</div><div>- // call the base destructor and then destructs any virtual bases.</div><div>- EmitGlobal(GlobalDecl(D, Dtor_Complete));</div><div><br></div><div>Just delegate this entire method to the ABI.</div><div><br></div><div><div dir="auto">+ /// Returns true if the given destructor type should be emitted as a linkonce</div><div dir="auto">+ /// delegating thunk, regardless of whether the dtor is defined in this TU or</div><div dir="auto">+ /// not.</div><div dir="auto">+ virtual bool useThunkForDtorVariant(CXXDtorType DT) const = 0;</div><div dir="auto"><br></div><div dir="auto">Go ahead and make this take the destructor decl as well.</div></div><div><br></div><div><div dir="auto">-bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {</div><div dir="auto">+bool CodeGenModule::MayDeferGeneration(const ValueDecl *D) {</div><div dir="auto"> // Never defer when EmitAllDecls is specified.</div><div dir="auto"> if (LangOpts.EmitAllDecls)</div><div dir="auto"> return false;</div><div dir="auto"> </div><div dir="auto">- return !getContext().DeclMustBeEmitted(Global);</div><div dir="auto">+ return !getContext().DeclMustBeEmitted(D);</div><div dir="auto"> }</div><div><br></div><div>This rename is now spurious.</div><div><br></div><div><div>+ if (getLangOpts().CPlusPlus && D && isa<CXXDestructorDecl>(D) &&</div><div>+ getCXXABI().useThunkForDtorVariant(GD.getDtorType()))</div><div>+ DeferredDeclsToEmit.push_back(GD);</div></div><div><br></div><div>Don’t bother with the language check.</div><div><br></div><div><div>+ // All MSVC dtors other than the base dtor are linkonce_odr and delegate to</div><div>+ // each other bottoming out with the base dtor. Therefore we emit non-base</div><div>+ // dtors on usage, even if there is no dtor definition in the TU.</div></div><div><br></div><div>This comment doesn’t need to be MSVC-specific. Just say “if the C++</div><div>ABI says this variant is a thunk, then we need to emit its definition on use.”</div><div><br></div><div><div> CodeGenModule::GetOrCreateLLVMFunction(StringRef MangledName,</div><div> llvm::Type *Ty,</div><div>- GlobalDecl D, bool ForVTable,</div><div>+ GlobalDecl GD, bool ForVTable,</div><div> llvm::AttributeSet ExtraAttrs) {</div></div><div><br></div><div>I agree with this rename, but please do it as a separate patch.</div><div><br></div><div>Otherwise, this looks great, thanks!</div><div><br></div><div>John.</div></div></div></body></html>