[PATCH] D128202: [clangd] Include "final" when printing class declaration

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 21 05:41:10 PDT 2022


sammccall added inline comments.


================
Comment at: clang/lib/AST/DeclPrinter.cpp:1010
 
+  if (D->isEffectivelyFinal()) {
+      Out << " final";
----------------
tom-anders wrote:
> sammccall wrote:
> > isEffectivelyFinal returns true for
> > `struct X { ~X() final; }`
> > 
> > I don't think we want to print `struct X final {}` in that case.
> > 
> > Probably want to replicate the check for FinalAttr instead?
> Agreed. Would probably make sense to add a `bool hasFinalAttribute() const` to `CXXRecordDecl`?
I'm not sure such a method pays for itself, the abstraction it might provides over hasAttr() is:
 - it handles the case where there's no definition... but maybe "return false" isn't the right thing to do in all circumstances?
 - a simple hasFinal() could avoid thinking about syntactic details - but here we explicitly care about them.
 - it hides the detail that final is stored as an attribute rather than e.g. a bit. This is the only piece that seems useful.

Given that, I think it's probably best here just to inline the logic, but I don't feel strongly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128202



More information about the cfe-commits mailing list