Mordante added a comment. Thanks a lot for working on this!
I wonder whether it would be better to have some of the more technical information in a separate file and let this file mainly have an introduction to modules in general and how to get started using them in Clang. I haven't tested the steps yet, but I intend to see whether the examples work for me. I haven't validated whether the module explanation is correct; I'm sure there are other reviewers who have a deeper knowledge of modules. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:11 + +Modules have a lot of meanings. For the users of clang compiler, modules may +refer to `Objective-C Modules`, `Clang C++ Modules` (or `Clang Header Modules`, ---------------- Clang is capitalized in its documentation. ================ 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 ---------------- Is `Modules_` intended to be a link? ================ Comment at: clang/docs/CPlusPlus20Modules.rst:20 +should be helpful to read Modules_ if you want to know more about the general +idea of modules. But due to the C++20 modules having very different semantics, it +might be more friendly for users who care about C++20 modules only to create a ---------------- ================ 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. + ---------------- IMO we don't need to justify why we add a new page. WDYT? ================ Comment at: clang/docs/CPlusPlus20Modules.rst:35 +tutorial. The document would only introduce concepts about the the +structure and building of the project. + ---------------- How about mentioning the state of modules in Clang? In your GitHub repository you mention some limitations. (A link to a status page could be an alternative.) ================ Comment at: clang/docs/CPlusPlus20Modules.rst:56 + +A module consists of one or multiple module units. A module unit is a special +translation unit. Every module unit should have a module declaration. The syntax ---------------- ================ 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. ---------------- Did you render the document? I would have expected double backticks. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:117 + +Here is a hello world example to show how to use modules. + ---------------- Maybe make this hello world example a bit simpler by not using partitions and have a separate example with partitions. This example gives the impression a simple "hello modular world" is a lot harder to achieve than a normal "hello world". ================ Comment at: clang/docs/CPlusPlus20Modules.rst:133 + module; + #include <iostream> + #include <string> ---------------- Does `import <iostream>;` work? If not is that a Clang or libc++ limitation? ================ 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 ---------------- Maybe explain what all steps do. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:178 + +We would explain the options in the following sections. + ---------------- ================ 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. ---------------- I assume `-std=c++2b` works too. Maybe rephrase the wording like this. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:184 +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. +The `-fmodules-ts` option is deprecated and is planned to be removed. ---------------- I would remove this sentence, it's not too common to backport language features. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:321 + +If the user don't want to follow the consistency requirement due to some reasons (e.g., distributing module files), +the user could try to use `-Xclang -fmodules-embed-all-files` when producing module files. For example: ---------------- ================ Comment at: clang/docs/CPlusPlus20Modules.rst:539 + +Since there is already one module maps in the source of libcxx. + ---------------- Maybe provide a link to this module map. (FYI the libc++ module map is generated by CMake using a `.in` file.) 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
