[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)

[James Y Knight via cfe-commits]

jyknight wrote:

>> The end result should be that #imported and #pragma once-guarded files are treated the same way as #ifndef-guarded files.

> While I don't necessarily disagree with that goal in principle, it wasn't true before this change either.

Well, yes, I know it isn't true yet -- that's exactly what I've been trying to say, and demonstrated with the test -- it's not true _and_ that's a bug.

> Modular headers including non-modular headers is a special problem. If we let every module enter the non-modular header (which Jan's patch does), its declarations will build into multiple modules/pcm files

I'm not really sure what you're getting at in this paragraph. Is there a _new_ special problem introduced here?

Yes, a textual header can be built into multiple modules/pcm files -- that's the nature of it being textual, right? Certainly it's always be a better to modularize headers from the bottom-up, and avoid textually including a header into multiple modules. And, yes, the performance cost of duplicating the decls into multiple modules' pcm files can be quite significant, depending on the size of the header. Yet, sometimes it happens. And, when it does, the declaration-merging takes care of it -- for C/C++ clients. This all works (and is used) today, with ifndef-guarded header inclusion.

But a textually-included header shouldn't be re-entered if it's been _visibly_ included already in the current compilation -- including if that was exported from an imported module. It should only be re-entered if the previous include is _not_ visible in the current context. Maybe you're saying Jan's patch doesn't do that properly yet. (I could see that being the case, since the behavior mostly just falls out automatically for the ifndef-guard case, based on whether the guard-macro was made visible or not!)

https://github.com/llvm/llvm-project/pull/83660