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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
