[clang] Llvm modules on demand bmi (PR #71773)

[Iain Sandoe via cfe-commits]

iains wrote:


> There are 2 things in the patch. One is to generate the BMI and the object file in one phase (phase here means preprocess, precompile, compile, ...). 

This is the main point of the patch - to do this efficiently.

>Another is to allow us to generate BMI from a `.cpp` file. (Currently we only do this for `.cppm` file or `-x c++-module` file).

Please do not become distracted by this, it is a side-effect of the main purpose, if the community wants to have modular files with some specific suffix, the patch still works the same.

> But after we introduced thin BMI, it looks inefficient to write the AST twice. So it is on my TODO list after we land the thin BMI patch. BTW, I think we should do thin in CodeGen action instead of hacking on WrappedASTConsumer.

I am curious as to why you think that the multiplex AST consumer is a hack - it seems to be designed exactly for this purpose and existed already (i.e. not part of this patch).


> For the second thing, I am curious that if it is necessary now? Or what will it block? I mean, the build systems or the cmake, require to mark the module unit ahead of time. Then the build systems will pass `-x c++-module` now for module units. Then the suffixes are not a thing for users.

I think you are getting distracted by the suffix; that is only a side-effect of this patch, not its primary purpose.


> And to me, the current mechanism for `.cppm` (or `-x c++-module`) in the driver side works pretty well. 

I have always viewed that as a temporary work-around because we could not generate both artefacts from one compiler invocation.

>And if we introduce the mechanism to produce BMI for `.cpp`, it implies that we need to maintain both paths. It is super embracing to me.

We do not need two mechanisms, .cppm can take the same path as any other suffix.

> > in the AST consumer on the BMI side doing suitable filtering to eliminate the content that is not part of the interface, that is either not needed (or in some cases positively unhelpful to consumers).

 
> I believe we should do this in ASTWriters.

I am strongly against doing more semantic work in the AST reader/writer; that is just compounding existing layering violations that are already hurting us.

> Also this should be part of thin BMI.

I am not sure what you mean here - the full AST is required for code-gen - we can only thin AST either on a separate path (as in this patch) or as a separate step.


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