[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:457 + +The declarations in a module unit which are not in global module fragment have new linkage names. + JohelEGP wrote: > Thanks for reviewing. BTW, the `request to change`

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 454012. ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 Files: clang/docs/CPlusPlus20Modules.rst

[PATCH] D131388: [docs] Add "C++20 Modules"

JohelEGP requested changes to this revision. JohelEGP added inline comments. This revision now requires changes to proceed. Comment at: clang/docs/CPlusPlus20Modules.rst:290 +``primary module interface unit`` by ``-fmodule-file`` +since the langugae specification says a module

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu added a comment. If no objection (not comments) come in, I want to land this in next Friday (8.25) since I want to backport this to 15.x and 15.x is going to release in September. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 453207. ChuanqiXu added a comment. Add description to use `-fmodule-header` to generate BMI for header units. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 Files: clang/docs/CPlusPlus20Modules.rst

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 452875. ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 Files: clang/docs/CPlusPlus20Modules.rst

[PATCH] D131388: [docs] Add "C++20 Modules"

ruoso added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:338-339 + +If we want to create libraries for the BMIs, we can't wrap these BMIs directly. +We must compile these BMIs(``*.pcm``) into object files(``*.o``) and wrap these object files. +

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu added a comment. In D131388#3719794 , @dblaikie wrote: > Actually, when it comes to diagrams - maybe what'd be good is a diagram of > classic compilation and a diagram of modules and header modules > > src1.cpp -+> clang++ src1.cpp -->

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 452612. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 Files: clang/docs/CPlusPlus20Modules.rst clang/docs/index.rst Index: clang/docs/index.rst

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 452610. ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 Files: clang/docs/CPlusPlus20Modules.rst

[PATCH] D131388: [docs] Add "C++20 Modules"

dblaikie added a comment. Actually, when it comes to diagrams - maybe what'd be good is a diagram of classic compilation and a diagram of modules and header modules src1.cpp -+> clang++ src1.cpp --> src1.o ---, hdr1.h --' +-> clang++ src1.o src2.o ->

[PATCH] D131388: [docs] Add "C++20 Modules"

ruoso added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:309 + +Remember to compile and link BMIs +~ I think this is a bit confusing. The BMI is not linked... Maybe something like: "Remember that modules still

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:395-396 + +Roughly, this theory is correct. But the problem is that it is too rough. Let's see what actually happens. +For example, the behavior also depends on the optimization level, as we will

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 452095. ChuanqiXu added a comment. Remove `non-inline` terms. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 Files: clang/docs/CPlusPlus20Modules.rst clang/docs/index.rst Index:

[PATCH] D131388: [docs] Add "C++20 Modules"

dblaikie added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:395-396 + +Roughly, this theory is correct. But the problem is that it is too rough. Let's see what actually happens. +For example, the behavior also depends on the optimization level, as we will

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:665 + # This is not allowed! + $ clang++ iostream.pcm -c -o iostream.o + dblaikie wrote: > could this use a strikethrough, perhaps? (not sure if you can strikethrough > inside a

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 451750. ChuanqiXu marked 2 inline comments as done. ChuanqiXu added a comment. Address comments: - Change `module file` to `BMI`. - Replace `rm -f` with `rm`. - Add a conclusion paragraph to the `How module speed up compilation` section. CHANGES SINCE

[PATCH] D131388: [docs] Add "C++20 Modules"

dblaikie added a comment. (I'll +1 to @ruoso's comments about using BMI - I think it might be best not to introduce a different term ("Module file") as it might confuse people as it's fairly close to "module source file", etc. BMI makes it clear/unambiguous that it's the binary file, not the

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu added a reviewer: dblaikie. ChuanqiXu added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:31 + +This document was intended to be a manual first and foremost, however, we consider it helpful to +introduce some language background here for readers who

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 451361. ChuanqiXu marked 13 inline comments as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 Files: clang/docs/CPlusPlus20Modules.rst

[PATCH] D131388: [docs] Add "C++20 Modules"

ruoso added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:70-76 +* Primary module interface unit. + +* Module implementation unit. + +* Module partition interface unit. + +* Module partition implementation unit. ChuanqiXu wrote: > ruoso wrote: >

[PATCH] D131388: [docs] Add "C++20 Modules"

dblaikie added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:31 + +This document was intended to be a manual first and foremost, however, we consider it helpful to +introduce some language background here for readers who are not familiar with

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:70-76 +* Primary module interface unit. + +* Module implementation unit. + +* Module partition interface unit. + +* Module partition implementation unit. ruoso wrote: > The

[PATCH] D131388: [docs] Add "C++20 Modules"

ruoso added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:225 + +It is possible to generate a module file for an importable module unit by specifying the ``--precompile`` option. + Likewise, here the term "Built Module Interface file", with the

[PATCH] D131388: [docs] Add "C++20 Modules"

ruoso added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:70-76 +* Primary module interface unit. + +* Module implementation unit. + +* Module partition interface unit. + +* Module partition implementation unit. The terminology here is a bit

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu updated this revision to Diff 451030. ChuanqiXu marked 2 inline comments as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 Files: clang/docs/CPlusPlus20Modules.rst

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu marked 8 inline comments as done. ChuanqiXu added a comment. In D131388#3706072 , @h-vetinari wrote: > Thanks! Repeating a point that might have been overlooked from D131062 > (this time not as comments in

[PATCH] D131388: [docs] Add "C++20 Modules"

iains added inline comments. Comment at: clang/docs/CPlusPlus20Modules.rst:673-674 + +Another reason is that there are proposals to introduce module mappers to the C++ standard (for example, https://wg21.link/p1184r2). +If we decide to reuse Clang's modulemap, we may get in

[PATCH] D131388: [docs] Add "C++20 Modules"

iains added a comment. general comment. Do we encourage contractions (don't, can't) etc. in documentation? I would suggest that to assist in any translation process it is better to write "do not" or "can not" instead (but that's just an opinion, not a matter of correctness).

[PATCH] D131388: [docs] Add "C++20 Modules"

h-vetinari added a comment. Thanks! Repeating a point that might have been overlooked from D131062 (this time not as comments in the diff to avoid the "pollution" that caused the move to this PR): > If you do open a new revision, please also consider breaking

[PATCH] D131388: [docs] Add "C++20 Modules"

ChuanqiXu created this revision. ChuanqiXu added reviewers: iains, urnathan, erichkeane, aaron.ballman, tahonermann, ilya-biryukov, Mordante, tschuett, philnik, cpplearner, h-vetinari. ChuanqiXu added projects: clang-modules, clang-language-wg, clang. Herald added a subscriber: arphaman. Herald