rsmith added inline comments.
================
Comment at: clang/lib/Lex/Pragma.cpp:1996
AddPragmaHandler("clang", new PragmaSystemHeaderHandler());
+ AddPragmaHandler("clang", new PragmaIncludeInsteadHandler());
AddPragmaHandler("clang", new PragmaDebugHandler());
----------------
Quuxplusone wrote:
> @rsmith wrote:
> > I'm not in love with the design of this feature. Basing the accept / warn
> > decision on whether the inclusion is in a system header seems fragile and
> > doesn't seem to enforce the intended semantics of the warning (that you can
> > only reach the implementation detail header through one of the named
> > headers), and limits the utility of the feature to system headers. As an
> > alternative, have you considered checking whether any of the headers named
> > in the pragma are in the include stack, and warning if not?
>
> @rsmith, I think you misunderstand part of the design goal here. You said
> "you can only reach the implementation detail header through one of the named
> headers" — this is true only for a maybe-surprising definition of "you." For
> example, if the user-programmer's `"MyThing.hpp"` tries to include
> `<__utility/move.h>` directly (or if IWYU wants to suggest how to include
> it), we expect the tool to suggest that you must include `<utility>`, not
> `<__utility/move.h>`. **However**, if the library's `<algorithm>` tries to
> include `<__utility/move.h>` directly, **that's** totally fine and expected.
> The entire point of these detail headers is to make sure that library headers
> don't have to `#include <utility>` when all they need is one little piece of
> it.
>
> So we do need **some** way to distinguish user-programmer headers like
> `"MyThing.hpp"` from library headers like `<algorithm>`. At the moment this
> is done by limiting the usefulness of the feature to just system headers, and
> making every system header a "friend" of every other system header.
>
> But one could imagine allowing a detail header to specify not only "here's
> how my non-friends should get to me" (e.g. by including `<utility>`) but also
> "here's a list of headers and/or directories which should be considered
> friends of mine" (and which could then include me directly, e.g.
> `<algorithm>` or `<__iterator/iter_move.h>`). We'd need a reliable syntax for
> that, though.
I think what you're suggesting is exactly what I suggested later: "We'd
presumably need two lists of headers: the external headers that we encourage
people to include to get at the implementation detail header, and the internal
list of headers that are also allowed to use it but that we shouldn't list in
the diagnostic."
[That was an edit I made after my original comment, though; I realized I forgot
to mention this and went back and added it. I guess your reply and my edit
happened at around the same time?]
As for syntax, something like this would make some sense to me:
```
#pragma clang include_instead(<foo>, "bar"; "private.h")
```
(Putting all the headers in a single pragma removes the need to track and check
this at the end of the file.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106394/new/
https://reviews.llvm.org/D106394
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
