[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ben.boeckel added a comment. FWIW, I was able to make CMake's module test suite pass with this patch on top of you MyP1689 branch on your Github fork. I also added some diffs to help clean up output files. I'll try and get it to work with the replacement patches as well, but I just want this to

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ben.boeckel added a comment. In D134267#3904257 , @ChuanqiXu wrote: > BTW, this patch should be discarded. And now we're pursuing D137059 > and D137058 > , where there is no modules ca

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3902564 , @ben.boeckel wrote: > I tried applying this patch to your MyP1689 branch (fixing conflicts with > attempts there), but it seems that something isn't being plumbed properly: > > clang-15: error: unknown a

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ben.boeckel added a comment. I tried applying this patch to your MyP1689 branch (fixing conflicts with attempts there), but it seems that something isn't being plumbed properly: clang-15: error: unknown argument: '-fc++-module-file-output=CMakeFiles/export_bmi_and_interfaces.dir/importable.pc

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3892629 , @ben.boeckel wrote: > In D134267#3876071 , @dblaikie > wrote: > >> I'm getting a bit exhausted with all the words involved here & not sure how >> to simplify/clar

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ben.boeckel added a comment. In D134267#3876071 , @dblaikie wrote: > I'm getting a bit exhausted with all the words involved here & not sure how > to simplify/clarify this. > > If @ben.boeckel has particular use cases, it might be easier for him to be >

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu abandoned this revision. ChuanqiXu added a comment. In D134267#3876071 , @dblaikie wrote: > I'm getting a bit exhausted with all the words involved here & not sure how > to simplify/clarify this. > > If @ben.boeckel has particular use cases, it

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. I'm getting a bit exhausted with all the words involved here & not sure how to simplify/clarify this. If @ben.boeckel has particular use cases, it might be easier for him to be here discussing them so we can discuss the tradeoffs directly rather than through intermedi

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. @dblaikie I am confusing about your concern. For the test coverage example, on the one hand, it is completely different from the case the coverage report was generated in the runtime instead of the compile time. On the other hand, it also provides a `-fprofile-dir` op

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. >> (I'd still sort of lean towards "make it the same as the .o, but with the >> .pcm suffix" and if the build system wants to put things in other places, it >> can move them around - but I understand why that's not something everyone's >> on board with) > > Actually, I

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu updated this revision to Diff 469136. ChuanqiXu added a comment. Use `-fc++-module-file-output` to address the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134267/new/ https://reviews.llvm.org/D134267 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3870064 , @ChuanqiXu wrote: > I grepped `options.td` and got (incomplete) list for options to take a output > name: > > # -o and its alias > -o > -object_file_name= > --output= > > /Fa (windows for assembly

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. I grepped `options.td` and got (incomplete) list for options to take a output name: # -o and its alias -o -object_file_name= --output= /Fa (windows for assembly output filename) /Fe (windows for output executable file name) /Fi (windows for preprocesse

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. Yeah, I am a little bit surprising/confusing to see why we're talking about the **behavior** of `-fmodule-file`. I guess the intention of @iains is to throw the option to prove `-fc++-module-file-path` (or `-fc++-module-file-output`) is the better. The point here is a

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3869678 , @dblaikie wrote: > In D134267#3869643 , @iains wrote: > >> In D134267#3869614 , @dblaikie >> wrote: >> >>> In D134267#3869162

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. In D134267#3869643 , @iains wrote: > In D134267#3869614 , @dblaikie > wrote: > >> In D134267#3869162 , @iains wrote: >> >>> In D134267#3868830 <

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3869614 , @dblaikie wrote: > In D134267#3869162 , @iains wrote: > >> In D134267#3868830 , @dblaikie >> wrote: >> >>> I'm OK with stickin

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. In D134267#3869162 , @iains wrote: > In D134267#3868830 , @dblaikie > wrote: > >> I'm OK with sticking with the existing `-fmodule-file` if that works for >> everyone. Yeah, it's short

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3869520 , @tahonermann wrote: >> In a pre-scanned world, the build system does know the info for each source >> file (published and dependent modules) [which ought to dispel some of the >> concerns raised about not kno

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

tahonermann added a comment. > In a pre-scanned world, the build system does know the info for each source > file (published and dependent modules) [which ought to dispel some of the > concerns raised about not knowing about possible outputs for > implementation/interface cases]. > > In a disco

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3868830 , @dblaikie wrote: > I'm OK with sticking with the existing `-fmodule-file` if that works for > everyone. Yeah, it's short and ambiguous in a space with many concepts of > what a "module file" is, but also the f

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. I'm OK with sticking with the existing `-fmodule-file` if that works for everyone. Yeah, it's short and ambiguous in a space with many concepts of what a "module file" is, but also the fact that it's a `-f` flag might help disambiguate it a bit - it's probably not the

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. > once again we are getting off topic for this patch :) Yeah, let's wait for @dblaikie's opinion on the flag name : ) The remained question: To use `-fc++-module-bmi-output=` or `-fc++-module-filename=`, or anything else (maybe `-fc++-module-output=`) CHANGES SINCE

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. once again we are getting off topic for this patch :) We (compiler engineers) will have to cater for multiple models used by different build systems. SG15 might give guidance/recommendations, but in the end the standard's normative text is not likely to make a 'discover

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3864416 , @ruoso wrote: > Currently, no. I think we do need a paper to discuss the requirements of the > remote execution protocol and how they relate to the implementation of C++ > modules. > > The simple summary i

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ruoso added a comment. Currently, no. I think we do need a paper to discuss the requirements of the remote execution protocol and how they relate to the implementation of C++ modules. The simple summary is that one way that remote execution is implemented is by just wrapping the compiler execu

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3864368 , @ruoso wrote: > FWIW, having a differently named argument for output files and input files is > very important for tooling. > > In particular, in the case of remote execution, it makes a huge difference >

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ruoso added a comment. FWIW, having a differently named argument for output files and input files is very important for tooling. In particular, in the case of remote execution, it makes a huge difference being able to tell what are the inputs and outputs by looking at the arguments alone, so I

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3864248 , @iains wrote: > I am also OK with doing this in two steps (first in the driver with this > patch and then by updating the FE to allow the two outputs from one > invocation - my draft patch series). For yo

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. > Yes, all of this stuff is important (and, yes, we are drifting into other > [related] topics) - we already have a mechanism for providing function bodies > to optimisation [LTO] - I do not think we should want to make module > interfaces larger than necessary to dup

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. I am also OK with doing this in two steps (first in the driver with this patch and then by updating the FE to allow the two outputs from one invocation - my draft patch series). BTW: I did mean to ask before .,, did you consider this (existing) command syntax? `-fmodule

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3863938 , @ChuanqiXu wrote: >>> Although modular code is user-facing - BMIs are an implementational detail, >>> right? >> >> but I don't think BMIs are an implementation detail, anymore than object >> files are - users

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added subscribers: ruoso, ben.boeckel. ChuanqiXu added a comment. >> Although modular code is user-facing - BMIs are an implementational detail, >> right? > > but I don't think BMIs are an implementation detail, anymore than object > files are - users should/will be as aware of BMIs as

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. In D134267#3851479 , @ChuanqiXu wrote: > In D134267#3849629 , @dblaikie > wrote: > >> To the original point I was making about implicit modules (which might've >> been my own confusion

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. (sorry I've not been following all this as closely as I should - but I don't think BMIs are an implementation detail, anymore than object files are - users should/will be as aware of BMIs as they are of .o files - there build artifacts that need to be cached, can be ou

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3861615 , @ChuanqiXu wrote: > Address @tschuett's opinion: > > - Use new introduced -fc++-module-cache-path= instead of -fc++-module-path to > avoid many logics about modules cache in clang modules. > - Use new introduce

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. @dblaikie @iains @tschuett @boris There are many opinions in this page. They are about problems in the higher level. They are pretty important and helpful indeed. I'm really appreciated it. But most of them are not related to this diff itself and I don't think they a

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu updated this revision to Diff 468138. ChuanqiXu added a comment. Address @tschuett's opinion: - Use new introduced `-fc++-module-cache-path=` instead of `-fc++-module-path` to avoid many logics about modules cache in clang modules. - Use new introduced `-fmodule-bmi-output=` instead of

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3854980 , @ChuanqiXu wrote: > In D134267#3852136 , @boris wrote: > >>> For example, my experimental support for P1689 >>> is at: [...]. The imple

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3852136 , @boris wrote: >> For example, my experimental support for P1689 >> is at: [...]. The implementation looks >> relatively trivial to me. The simplicity is pretty important. >

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. To avoid more tangential discussion here - I've opened https://discourse.llvm.org/t/generating-pcm-module-interfaces-and-regular-object-files-from-the-same-compiler-invocation/65918 ... it would be great if the major stakeholders here especially @rsmith could comment on

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

boris added a comment. > For example, my experimental support for P1689 > is at: [...]. The implementation looks > relatively trivial to me. The simplicity is pretty important. Two points: 1. It's worth considering the simplicity of the overall system (compiler

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3852047 , @boris wrote: > Just to add a few points on the module mapper discussion: overall, the > current view is that there are two ways to handle C++20 modules from the > build system's POV: (1) pre-scan the code

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

boris added a comment. Just to add a few points on the module mapper discussion: overall, the current view is that there are two ways to handle C++20 modules from the build system's POV: (1) pre-scan the codebase to figure out what is what and who depends on whom ahead of compilation and (2) mo

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a reviewer: tschuett. ChuanqiXu added a comment. In D134267#3849629 , @dblaikie wrote: > To the original point I was making about implicit modules (which might've > been my own confusion due to the term being brought up in D134269 >

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3849712 , @dblaikie wrote: > In D134267#3849673 , @iains wrote: > >> >> - I do not think this patch fully addresses the issue of harmonising GCC and >> clang's command lines

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. In D134267#3849673 , @iains wrote: > (trying not derail this discussion further) > > - Yes, the alternate solution proposed for the "Hello World" case would work > with the module mapper interface. However, the initial discussi

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

tschuett added a comment. The one phase compilation should work with `-fno-implicit-modules`! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134267/new/ https://reviews.llvm.org/D134267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

tschuett added a comment. What really worries me that you are talking about one phase compilation and there is a module cache. Even if it is one phase compilation you must pass all dependent modules on the command line. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134267/new/ https:/

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. (trying not derail this discussion further) - Yes, the alternate solution proposed for the "Hello World" case would work with the module mapper interface. However, the initial discussion was simply about being able to produce both the .PCM and .O artefacts from one invoc

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. To the original point I was making about implicit modules (which might've been my own confusion due to the term being brought up in D134269 ): Implicit modules is the situation where the compiler, finding that it needs a module while

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3848974 , @ChuanqiXu wrote: > In D134267#3848958 , @iains wrote: > >> In D134267#3848883 , @ChuanqiXu >> wrote: >> >>> In D134267#384887

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3848958 , @iains wrote: > In D134267#3848883 , @ChuanqiXu > wrote: > >> In D134267#3848876 , @iains wrote: >> >>> If we are going to

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3848883 , @ChuanqiXu wrote: > In D134267#3848876 , @iains wrote: > >> If we are going to **//require//** the serialisation/deserialisation for >> correctness then IMO we should h

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3848876 , @iains wrote: > If we are going to **//require//** the serialisation/deserialisation for > correctness then IMO we should have an error for an incorrect compilation. Of course. > I still feel that deseria

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3848862 , @ChuanqiXu wrote: > In D134267#3848793 , @iains wrote: > >> In D134267#3848745 , @ChuanqiXu >> wrote: >> FAOD: I think th

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3848822 , @tschuett wrote: > Clang header modules started with implicit builds and module caches. Apple is > moving too explicit builds. There are all kinds of problems with implicit > module builds. I believe that

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3848793 , @iains wrote: > In D134267#3848745 , @ChuanqiXu > wrote: > >>> FAOD: I think this should not be about "who's patches" - but about the >>> compilation model and com

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

tschuett added a comment. In D134267#3848838 , @iains wrote: > In D134267#3848822 , @tschuett > wrote: > >> Clang header modules started with implicit builds and module caches. Apple >> is moving too explicit bu

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3848822 , @tschuett wrote: > Clang header modules started with implicit builds and module caches. Apple is > moving too explicit builds. There are all kinds of problems with implicit > module builds. I believe that C++2

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

tschuett added a comment. Clang header modules started with implicit builds and module caches. Apple is moving too explicit builds. There are all kinds of problems with implicit module builds. I believe that C++20 modules shouldn't make the same mistake. CHANGES SINCE LAST ACTION https://rev

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3848745 , @ChuanqiXu wrote: >> FAOD: I think this should not be about "who's patches" - but about the >> compilation model and command lines that we want to end up with. > > BTW, in the current context, when I say "my" a

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. > FAOD: I think this should not be about "who's patches" - but about the > compilation model and command lines that we want to end up with. BTW, in the current context, when I say "my" and "your", I just want to refer the corresponding things. There is no means to off

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3848539 , @ChuanqiXu wrote: > In D134267#3847399 , @iains wrote: > >> >> I am concerned that with the current scheme (with or without the change in >> this review) we could e

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3847399 , @iains wrote: > FAOD: I think this should not be about "who's patches" - but about the > compilation model and command lines that we want to end up with. Yeah, of course. > I am concerned that with the cu

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. FAOD: I think this should not be about "who's patches" - but about the compilation model and command lines that we want to end up with. I am concerned that with the current scheme (with or without the change in this review) we could end up with three process launches per

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3846264 , @iains wrote: > In D134267#3846218 , @ChuanqiXu > wrote: > >> In D134267#3846153 , @iains wrote: >> >>> @ChuanqiXu BTW, I thin

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3846264 , @iains wrote: > In D134267#3846218 , @ChuanqiXu > wrote: > >> In D134267#3846153 , @iains wrote: >> >>> @ChuanqiXu BTW, I

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. In D134267#3846218 , @ChuanqiXu wrote: > In D134267#3846153 , @iains wrote: > >> @ChuanqiXu BTW, I think we (unfortunately) have some overlap in work in >> progress here; >> >> I have imple

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3846153 , @iains wrote: > @ChuanqiXu BTW, I think we (unfortunately) have some overlap in work in > progress here; > > I have implemented a similar scheme to GCC's where C/BMIs can be produced "on > demand" when the

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

iains added a comment. @ChuanqiXu BTW, I think we (unfortunately) have some overlap in work in progress here; I have implemented a similar scheme to GCC's where C/BMIs can be produced "on demand" when the FE encounters the module keywords. The idea was to make it possible to have the same com

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a comment. In D134267#3844605 , @ChuanqiXu wrote: > In D134267#3815453 , @dblaikie > wrote: > >>> This also tries to fix the problem I raised a year ago: >>> https://discourse.llvm.org/t/make-com

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a comment. In D134267#3844605 , @ChuanqiXu wrote: > In D134267#3815453 , @dblaikie > wrote: > >>> This also tries to fix the problem I raised a year ago: >>> https://discourse.llvm.org/t/make-comm

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu added a reviewer: rsmith. ChuanqiXu added a comment. In D134267#3815453 , @dblaikie wrote: >> This also tries to fix the problem I raised a year ago: >> https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/591

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

dblaikie added a subscriber: rsmith. dblaikie added a comment. > This also tries to fix the problem I raised a year ago: > https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144 I think this thread is fairly different from what this patch is proposing. @rs

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu updated this revision to Diff 462700. ChuanqiXu added a comment. - Add ReleaseNotes. - Refine the behavior about partitions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134267/new/ https://reviews.llvm.org/D134267 Files: clang/docs/ReleaseNotes.rst clang/include/clang/B

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ChuanqiXu created this revision. ChuanqiXu added reviewers: iains, dblaikie, Bigcheese, vsapsai. ChuanqiXu added projects: clang, clang-modules. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. This patch tries to make clang