rnk marked an inline comment as done. rnk added inline comments.
================ Comment at: clang/include/clang/AST/Attrs.h:1 +//===--- Attr.h - Classes for representing attributes ----------*- C++ -*-===// +// ---------------- aaron.ballman wrote: > 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? That makes sense. Do you mind if I land this first, and then do the Attr.h -> AttrBase.h rename? That way I don't have to make a new patch, put it up for review, and rebase this patch over it. I'll send a review assuming the answer is yes and then reorder them if you have concerns. 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
