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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
