[PATCH] D125061: [clang] A more robust way to attach comments

Argyrios Kyrtzidis via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 5 21:02:23 PDT 2022


akyrtzi added a comment.

Since the issue is specific to enumerators I would recommend against increasing the size of `DeclBase`, which would increase the size of every single `Decl` in the AST.

There are multiple ways to approach this:

- Consider whether bringing back the comma check just for enumerators would work.
- Check the enumerators from the parent `EnumDecl`. The number of enumerators inside an `EnumDecl` is usually relatively small, you can do measurements to see how the performance looks with actual code.
- If there is noticable performance regression:
  - Either add "previous enumerator" pointer in `EnumConstantDecl` (increases size of every `EnumConstantDecl` in the AST)
  - Or add "out-of-AST-decl" tracking via adding some `DenseMap` in `ASTContext` that is only populated when comments for enumerators of an `EnumDecl` are requested. There is much precedent for this, for example see these examples:

  	  /// A cache mapping from CXXRecordDecls to key functions.
  	  llvm::DenseMap<const CXXRecordDecl*, LazyDeclPtr> KeyFunctions;
  
  	  /// Mapping from ObjCContainers to their ObjCImplementations.
  	  llvm::DenseMap<ObjCContainerDecl*, ObjCImplDecl*> ObjCImpls;
  
  	  /// Mapping from ObjCMethod to its duplicate declaration in the same
  	  /// interface.
  	  llvm::DenseMap<const ObjCMethodDecl*,const ObjCMethodDecl*> ObjCMethodRedecls;
  
  	  /// Mapping from __block VarDecls to BlockVarCopyInit.
  	  llvm::DenseMap<const VarDecl *, BlockVarCopyInit> BlockVarCopyInits;
  	  

The benefit of a separate map is that it only takes additional memory when a particular functionality is requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125061



More information about the cfe-commits mailing list