[PATCH] D68795: [libTooling] Introduce a single, "main" header file for Transformer.

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 10 09:54:50 PDT 2019


ymandel marked an inline comment as done.
ymandel added inline comments.


================
Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:28
+// also update their code to reference `TransformerTool`.
+#include "clang/Tooling/Transformer/TransformerTool.h"
+// Temporary alias to assist clients in migrating to new class name.
----------------
gribozavr wrote:
> I'm not a fan of umbrella headers... especially if they don't cover everything.
> 
> I think "include what you use" is a better rule.
> 
> The rest of the patch makes sense to me, just not this file. (OK to keep it temporarily if we need it to help clients migrate, of course.)
Thanks, sounds good.  In that case, might it make more sense to just split off RewriteRule and not rename Transformer -> TransformerTool (along w/ the correpsonding header change)?  My primary motivation for renaming the class was to allow Transformer to be the name for the umbrella.

Since Transformer.h will have to include RewriteRule.h anyhow, there won't be any need for a forwarding header while we update users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68795





More information about the cfe-commits mailing list