[PATCH] D136071: [include-cleaner] Add PragmaIncludes which handles include-mapping pragmas.

Mon Oct 31 03:09:22 PDT 2022

sammccall added inline comments.

================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:41
+// spelling header rather than the header directly defines the symbol.
+class PragmaIncludes {
+public:
----------------
hokein wrote:
> kadircet wrote:
> > i think this interface is OK, if we're planning to implement another adapter on top of this class and use this only as a repository. Then have the adapter provide a more convenient interface/data structures for types of queries we'll have during `Location => Header` mapping inside include cleaner.
> > 
> > As those will most likely look like `getHeaders(SourceLocation)/getHeaders(string)` -> `vector<pair<Header, Signals>>`, without really caring about the details an implementation detail being mapped to some other file, or getting exports of a certain header (as discussed this is the piece that would require a little bit more detailed design).
> yeah, the plan is that this class is a low-level piece, only providing the data extracted from pragmas, and we will build another layer on top of this class (this is my next step).
> 
My suggestion would be to see what it looks like coding directly against this first - it might be fine, or it might make it obvious that making this class a little higher level is a better choice.
Trying to place an abstraction around this kind of "bits and pieces" information is easy to get wrong ime

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136071