ChuanqiXu added a comment. In D134267#3848793 <https://reviews.llvm.org/D134267#3848793>, @iains wrote:
> In D134267#3848745 <https://reviews.llvm.org/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" and "your", I just want to >> refer the corresponding things. There is no means to offend. > > sure - no problem. > > I guess we ought to be a bit more clear: > There is implementation divergence between GCC and clang and from the point > to view of users and build systems having two different command line sets is > not helpful. > > The patches I drafted were aimed at removing that divergence. > If that is not possible (because the ODR analysis requires steps that make it > very difficult or inefficient to do so) then some more thought and/or > restructuring is needed. Yeah, now this is more clear. >> In D134267#3848672 <https://reviews.llvm.org/D134267#3848672>, @iains wrote: >> >>>> (2) Your scheme saves the time for deserialization. However, clang >>>> implement many ODR checks in the deserialization part. Skipping the >>>> deserialization will omit these ODR checks, which will break the >>>> semantics. And this is the reason why that scheme may not be good. >>> >>> What you seem to be saying here is that: >>> >>> `clang++ -std=c++20 foo.cxxm -c -o foo,o` >>> >>> edit; maybe I should have written -xc++ foo.cxxm >>> >>> Does different ODR analysis from: >>> >>> `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm` >>> `clang++ -std=c++20 foo.pcm -o foo.o` >>> >>> If so, then that is of some concern since those are both valid compilations >>> in the current compiler. >>> >>> In the first case, the back end is connected as an AST consumer to the >>> front - there is no serialise/deserialise. >> >> No. **Currently**, in clang, the following two is the same: >> >> `clang++ -std=c++20 foo.cxxm -c -o foo.o` > > That's why I added the edit ... > > `clang++ -std=c++20 -xc++ foo.cxxm -c -o foo.o` > > AFAIU that will not produce any error for the user? > > suppose I have foo.cxx which includes modules and has local declarations? > `clang++ -std=c++20 foo.cxx -c -o foo.o` > >> with >> >> `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm` >> `clang++ -std=c++20 foo.pcm -o foo.o` >> >> The first compilation will generate `.pcm` files in the `/tmp` directories >> and compile the `.pcm` files in the `/tmp` directories. > > yes, I should have added the -xc++ in the first case ;) Oh, I got it. Now it makes sense. I misses the edit : (. The answer is: (1) Now, the behavior is different. And I once sent a bug report for it. (2) **Now** there won't be direct error message. And I **was** planned to add it. This is the reason why I closed the above bug report : ) (3) If we're going to make it an error, this is not decided yet. >> We could verify it by inserting debugging informations. Or writing a >> ODR-mismatch example, then when the compiler emits an error, we'll find the >> compiler emits the error in the deserialization part. I met these 2 >> situations for many times : ) >> >>> The model I implemented simply streams the AST instead of throwing it away >>> ... >> >> So that model misses the ODR checks, which is not good. Since it matters >> with the semantics. > > Understood - I wonder if we have layering right here - surely Sema should be > responsible for ODR checks? > Is there any reason that relevant hashes cannot be generated on the AST > //**without**// streaming it? > Was this model intended from the start ? > (the fact that one can connect a direct AST consumer to the back end, > suggests that enforcing serialisation/deserialisation was not expected to be > a requirement at some stage) In fact, we do the ODR checks in Sema too. (Would you feel it amazing? I felt amazing when I realize this at the first time) For the following example, the ODR check is made in Deserializer: module; #include "something" export module any_module_name; import m; or // decls import m; or import m1; import m2; In the above cases, the ODR check is made in deserialization part. And in the following examples, the ODR check is made in Sema part: import m; // decls What is the principle? I think it is caused by the on-the-fly compilation principle (The name is constructed by myself. I think you know what I am saying.) I mean, when we (the compiler) parse a declaration and we want to know if it violates the ODR, we made it in the Sema part. And when we import something (precisely, read something from a module, since clang did lazy loading), we did the ODR check in deserialization. Although I felt odd at first, but now I feel it reasonable in some levels. How do you feel now? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134267/new/ https://reviews.llvm.org/D134267 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
