vsapsai added a comment.
In D104344#2969178 <https://reviews.llvm.org/D104344#2969178>, @jansvoboda11
wrote:
> If my understanding is correct, this patch only changes how we track
> `HeaderFileInfo::{isImport,NumIncludes}` and makes sure to store the
> necessary info into AST files. Right?
Yes, you are correct.
> Can you explain how values of these members can prevent Clang from importing
> the header and how this interacts with visibility?
Currently, in AST we basically store the state of `HeaderSearch` after building
a module. And `isImport` means "if //any// of the headers in the module has
used #import", `NumIncludes` means "how many times //all// of the headers in
the module have included the given header". So in the test case
import-nonmodular.m after building the module 'Modularized' we have for the
header Unmodularized.h `isImport = true, NumIncludes = 1`. So when we see
`#import <Unmodularized/Unmodularized.h>` we decide not to enter the header
because according to PCM data it was already imported once. But when we try to
use UnmodularizedStruct, we cannot do that because it is present in
Modularized.Invisible but unfortunately not visible.
So my fix is to track `HeaderFileInfo::{isImport,NumIncludes}` per including
submodule. And when we encounter include/import in the source code, try to load
ExternalHFI from ExternalSource, we use `isImport` and `NumIncludes` only from
the modules that are [transitively] visible at this point. This way we don't
use `isImport` and `NumIncludes` from irrelevant modular headers.
Another option is to treat unmodularized headers as submodules and update their
visibility when we see `#import <Unmodularized/Unmodularized.h>`. But that's
pretty complicated and I'm not sure it is possible when we have the same
non-modular header included from different modules. Then we have multiple
copies of the same fake submodule and it's not clear which one to use, which
one to update.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104344/new/
https://reviews.llvm.org/D104344
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
