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

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 28 10:23:37 PDT 2021


dblaikie added a comment.

In D106394#2910571 <https://reviews.llvm.org/D106394#2910571>, @cjdb wrote:

> In D106394#2905660 <https://reviews.llvm.org/D106394#2905660>, @dblaikie wrote:
>
>> Just a thought (nothing to hold up this patch/suggest a revert/suggest any immediate action), but:
>>
>>> The problem with extending this to non-system headers is that you need a way to tell which headers are allowed to include the detail headers and which ones are not.
>>
>> What if the analysis was done on the entire #include path from the current primary source file - rather than from the immediate include location? Then the requirement could be "This header must be directly or indirectly included from one of the headers listed in include_instead, otherwise there will be a warning" - could that generalize to other use cases/not only system headers?
>
> I think this would mean that something like libc++'s `<__ranges/concepts.h>` couldn't directly include `<__iterator/concepts.h>`?

Ah, I think I follow what you're getting at - I think you're suggesting that the include_insteads are not intended to be exhaustive? (they're not all the ways you might get these declarations by including public headers - they are only meant to enumerate the public/intended entry points to these declarations)

So despite the fact that `ranges` indirectly includes `__iterator/concepts .h`, you wouldn't want to tell the user that they could include `ranges` to get the declarations in `__iterator/concepts.h` ?

Because `__iterator/concepts.h` is only needed as an implementation detail of `ranges`, not as part of its public interface? Fair enough.

Pity - if the enumeration of public headers that can indirectly include an implementation header was complete, then that would be sufficient to implement a warning without needing the system header detection, I think? So one option would be to annotate the include_instead with "implementation detail" V "public interface" bit/distinction and that would work instead of the system header detection? But I guess that'd be left to another project/further work to generalize this feature.

> Also, how does this play with `#pragma GCC system_header` detection?

I don't think my suggestion would have anything to do with system header detection, so far as I can think of - could you describe this concern in more detail?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106394



More information about the cfe-commits mailing list