sammccall added a comment. In D106394#2893457 <https://reviews.llvm.org/D106394#2893457>, @ldionne wrote:
> In D106394#2892832 <https://reviews.llvm.org/D106394#2892832>, @sammccall > wrote: > >> Eventually this seems like a reasonable thing to want for user code. What >> are your motivations for restricting it to system headers? (And do you think >> people in future will be tempted to lift them?) > > 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. Ah, of course. It's a shame if this is a fatal reason because it seems like a small thing. IWYU defines a whole bunch of other "pragmas", and one is "IWYU pragma: friend" for this purpose. This works but adds complexity to the source code (I didn't see it being as heavy as defining modules, but I haven't thought about it much) and also to consuming tools (we don't support it in clangd for this reason). Annotating the include site instead like `#include "internals.h" // friend` seems almost tempting but magic-comments issues aside doesn't solve all the questions tools have (like what include to insert if none exists yet). > Hmm... I like prior art. That clangd supports it suggests that there's a > section of code I can look at for inspiration if we were to replace this > pragma with the IWYU comment-pragma Sure, there's nothing terribly interesting to see though. Clangd doesn't yet use it for warnings, but sometimes when we encounter an indexed symbol we consider inserting an `#include` for it (code completion, fixit for missing symbol). At indexing time the IWYU pragma is used to determine which insertable header we'll record for that symbol. Indexing 1 <https://github.com/llvm/llvm-project/blob/f14495dc75d72d8900b3f4056a36a1ba5063e957/clang-tools-extra/clangd/index/CanonicalIncludes.cpp#L59-L82> - parse an IWYU comment. Note that it always produces a quoted string, which means a verbatim `#include` spelling, rather than a resolved file path, thus the FIXME. Indexing 2 <https://github.com/llvm/llvm-project/blob/f14495dc75d72d8900b3f4056a36a1ba5063e957/clang-tools-extra/clangd/index/SymbolCollector.cpp#L228-L255> - determine the includable header for symbols from a FileID. (The code completion and diagnostic fixit code isn't terribly interesting or readable...) > (I wonder why they didn't just go with #pragma IWYU ...?). Historically, it may have been the principled reason @Quuxplusone mentioned, or it may simply have been that as an *out-of-tree* clang-based tool, it's easier to hook comments in PPCallbacks than it is to hook an unknown pragma. (Of course these reasons are connected) > If IWYU has already developed a de facto standard for this information, I > would recommend libc++ just use it (perhaps with no changes needed in clang > at all). One hesitation I'd have about adopting this convention is that while the "IWYU pragma: friend" subset seems to coincide exactly with the design you want, the full set of IWYU pragmas are a rich/complicated set that I'm less confident in the design of. Supporting only this subset will cause *some* confusion/pressure for more, but later choosing to extend the model will force a choice between adopting more IWYU pragmas (may be the wrong design) or doing something else with overlapping functionality (lots of confusion). 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
