erichkeane 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(); ---------------- rnk wrote: > 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. Thanks! That is at least more descriptive that a virtual function in a non-inheriting final type is intentional and not just silliness. I appreciate the change! 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
