ChuanqiXu added inline comments.
================ Comment at: clang/docs/CPlusPlus20Modules.rst:18 + +There is already a detailed document about clang modules Modules_, it +should be helpful to read Modules_ if you want to know more about the general ---------------- Mordante wrote: > Is `Modules_` intended to be a link? Yes, it is linked to the clang modules page: https://clang.llvm.org/docs/Modules.html. This mimics the style of https://clang.llvm.org/docs/Modules.html#where-to-learn-more-about-modules ================ Comment at: clang/docs/CPlusPlus20Modules.rst:22 +might be more friendly for users who care about C++20 modules only to create a +new page. + ---------------- Mordante wrote: > IMO we don't need to justify why we add a new page. WDYT? I guess it is not bad to add it. Since it is really confusing about clang modules and C++20 Modules at first for people who are not so familiar to these topics. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:79 +A primary module interface unit is a module unit whose module declaration is +`export module module_name;`. The `module_name` here denotes the name of the +module. A module should have one and only primary module interface unit. ---------------- Mordante wrote: > Did you render the document? I would have expected double backticks. Yeah, I had rendered it in https://overbits.herokuapp.com/rsteditor/. But it is not good. I found http://rst.aaroniles.net which looks better. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:133 + module; + #include <iostream> + #include <string> ---------------- Mordante wrote: > Does `import <iostream>;` work? If not is that a Clang or libc++ limitation? Yes, `import <iostream>` works here. But I don't want to introduce too many information here. And I mentioned it indirectly later in `Include translation` section. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:164 + # Precompiling the module + clang++ -std=c++20 interface_part.cppm --precompile -o M-interface_part.pcm + clang++ -std=c++20 impl_part.cppm --precompile -fprebuilt-module-path=. -o M-impl_part.pcm ---------------- Mordante wrote: > Maybe explain what all steps do. I made a simple explanation in the `#` part and I intended to explain them in more detail in the following sections. I feel it may be confusing for readers who reads at the first time? ================ Comment at: clang/docs/CPlusPlus20Modules.rst:183 + +Currently, C++20 Modules is enabled automatically if the language standard is `-std=c++20`. +Sadly, we can't enable C++20 Modules now with lower language standard versions like coroutines by `-fcoroutines-ts` due to some implementation problems. ---------------- Mordante wrote: > I assume `-std=c++2b` works too. Maybe rephrase the wording like this. I sent a backport patch to 15.x: https://github.com/llvm/llvm-project/issues/56803. So it `-std=c++2b` should be fine in 15.x CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131062/new/ https://reviews.llvm.org/D131062 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
