[PATCH] D139653: [clang] Set ShowInSystemHeader for module-build and module-import remarks

Thu Dec 8 13:33:49 PST 2022

jansvoboda11 accepted this revision.
jansvoboda11 added subscribers: vsapsai, Bigcheese.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

In D139653#3982272 <https://reviews.llvm.org/D139653#3982272>, @kastiglione wrote:

>> Also, do you think this should be configurable on the command line?
>
> Do you have a flag in mind?

We could have a separate `-Ruser-module-{build,import}` (and maybe `-Rsystem-module-{build,import}`) besides the `-Rmodule-{build,import}` (that would now include both).

>> I'm thinking about users that are trying to debug their own modular code, but don't really care what happens in the SDK.
>
> My first question is how many of those users are there? It seems to me that the module remarks are more for tooling developers than end users.

I'm not sure TBH. But I think Clang modules are used by people that are not familiar with the inner workings of the SDK.

> Speaking as someone working on tools, I think I would always want all module-build remarks, since – correct me if I'm wrong – they're relevant only to performance, and for performance I want to see all the builds.

FWIW, these remarks have been useful for me when checking correctness of implicit builds as well. As for performance, let's pretend I'm trying to optimize LLVM's modular build. I can imagine myself starting investigating our own modules (e.g. seeing how many times `clangFrontend` gets built), and only dropping down into system headers once that's optimized. Getting bombarded by all the modules that are built for the SDK right away would be too noisy.

> For module-import remarks, I could see it being more interesting to developers, but I don't know how many developers know about and use `-Rmodule-import`. If we want to proactively support those users, maybe the solution is two flags, say `-Rmodule-import` and `-Rmodule-import-all` (I can't think of a better spelling).
>
> For relevant comparisons, the only thing I can think about is the `-H` flag, which shows all header includes, it does not exclude system headers. Speaking of which, an equivalent for modules might be helpful.

I'm fine with landing current functionality provided it's tested (and people like @vsapsai and @Bigcheese don't have objections).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139653