[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 18 04:51:55 PST 2022


hokein added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:47
+  }
+  void buildAST() { AST = std::make_unique<TestAST>(Inputs); }
+
----------------
kadircet wrote:
> nit: i'd actually rename this to `astWithIncludes` and take in a set of filenames, then set up `Inputs.Code` here, eg:
> ```
> astWithIncludes({"foo.h", "bar.h"}); -> Inputs.Code = `#include "foo.h"\n#include "bar.h"`; AST = TestAST(Inputs);
> ```
I think hiding and synthesizing the code of main file inside a function will hurt the code readability (though the main-file code is boilerplate). As a reader, I will prefer much that the main code and the all #includes are defined in a single place which is inside the TEST_F. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698



More information about the cfe-commits mailing list