hokein 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:
----------------
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).
================
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:54
+
+ class RecordPragma;
+
----------------
kadircet wrote:
> why do we need to expose this?
no needed, move it to private.
================
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:67
+ llvm::DenseMap<llvm::sys::fs::UniqueID, std::string /*VerbatimSpelling*/>
+ IWYUPrivate;
+
----------------
sammccall wrote:
> nit: naming a member after the keys rather than the values is confusing,
> consider `IWYUPublic` or just `PublicMapping`?
rename to `IWYUPublic`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136071/new/
https://reviews.llvm.org/D136071
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
