[PATCH] D70627: [AST] Split out Attrs.h for table generated *Attr classes

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 25 13:14:44 PST 2019


aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/Attrs.h:1
+//===--- Attr.h - Classes for representing attributes ----------*- C++ -*-===//
+//
----------------
rnk wrote:
> aaron.ballman wrote:
> > This should read `Attrs.h` instead, but I find the naming distinction to be a bit too subtle. How about `AttrSubclasses.h` or `ConcreteAttrs.h` (other suggestions welcome too).
> WDYT about following Decl.h / DeclBase.h? So, rename Attr.h to AttrBase.h, update all current includers of Attr.h to AttrBase.h, then add in new includes of Attr.h. Perhaps staged as two commits, one to do the rename, one to introduce the new header.
> 
> Otherwise, I guess I like AttrSubclasses.h from your list, or AttrClasses.h as an alternative.
I think AttrBase.h for the `Attr` base class makes a lot of sense -- it makes it more clear that you only get the base class. WDYT about `Attrs.h` for the subclasses instead of `Attr.h` -- might make it a bit more clear that you're getting all of the attributes, not just the base class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70627





More information about the cfe-commits mailing list