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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
