[PATCH] D104344: [modules] Track how headers are included by different modules.

[Volodymyr Sapsai via Phabricator via cfe-commits]

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