rnk added a subscriber: akhuang.
rnk added inline comments.

================
Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();
----------------
erichkeane wrote:
> rnk wrote:
> > hans wrote:
> > > I worry that this is going to look obscure to most readers passing 
> > > through. Maybe it could be expanded to more explicitly spell out that it 
> > > reduces the size of the debug info?
> > I want to keep it concise, most readers shouldn't need to know what this 
> > is, and they can look up technical terms like "key method". I'll say "debug 
> > info" instead of "type info", though, that should be more obvious.
> FWIW, I just ran into this and did a double/triple take, as it didn't make 
> sense for me to see a 'virtual' function in a 'final' type that didn't 
> inherit to anything looked like nonsense.
> 
> The only way I found out what this meant (googling "key method" did very 
> little for me here) was to do a 'git-blame' then found this review.  The ONLY 
> place that explained what is happening here is the comment you made here: 
> https://reviews.llvm.org/D70340#1752192
Sorry, I went ahead and wrote better comments in 
rG6d78c38986fa0974ea0b37e66f8cb89b256f4e0d.

Re: key functions, this is where the idea is documented:
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable
They control where the vtable is emitted. We have this style rule to take 
advantage of them:
https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers
However, the existing rule has to do with RTTI and vtables, which doesn't make 
any sense for Sema.

The idea that class debug info is tied to the vtable "known", but not well 
documented. It is mentioned maybe once in the user manual:
https://clang.llvm.org/docs/UsersManual.html#cmdoption-fstandalone-debug
I couldn't find any GCC documentation about this behavior, so we're doing 
better. :)

@akhuang has been working on the constructor homing feature announced here:
https://blog.llvm.org/posts/2021-04-05-constructor-homing-for-debug-info/
So maybe in the near future we won't need this hack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to