[PATCH] D106394: [clang][pp] adds '#pragma include_instead'

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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