[PATCH] D54986: Make CodeGen choose when to emit vtables.

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 29 13:46:05 PST 2018


rjmccall added inline comments.


================
Comment at: lib/AST/RecordLayoutBuilder.cpp:3069
   const Decl *Result =
       Entry ? Entry.get(getExternalSource()) : computeKeyFunction(*this, RD);
 
----------------
rsmith wrote:
> rjmccall wrote:
> > Why not just change `computeKeyFunction` to decide that the class doesn't have a key function if it would be an inline definition?  We can do the same updating we do with the ARM rule except just bailing out instead of scanning for another candidate.
> It seems desirable to me for Clang's notion of what the "key function" is to exactly match the ABI specification. That way, the decision to not emit a vtable with an inline key function is purely a lowering concern, not part of the program model represented by the frontend, and non-compiler tools who query this data will get correct answers. Also, if we made the suggested change, I'd still want to perform the linkage check to avoid unnecessary emission of vtables for explicit instantiation definitions, so this wouldn't remove any complexity from CodeGen.
> 
> If we want to go down this path, I'd prefer that we also change the ABI document to say that an inline virtual function definition results in the class having no key function.
That's fair.  We could instead wrap all the calls to the function from IRGen to apply this rule retroactively; that should be easy.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54986/new/

https://reviews.llvm.org/D54986





More information about the cfe-commits mailing list