[PATCH] D143587: [Docs] Multilib design

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 10 22:39:10 PST 2023


MaskRay added inline comments.


================
Comment at: clang/docs/Multilib.rst:41
+
+The available libraries can be hard-coded in clang. Typically this is done
+using the ``MultilibBuilder`` interface. There are many examples of this in
----------------
`s/clang/Clang/`


================
Comment at: clang/docs/Multilib.rst:42
+The available libraries can be hard-coded in clang. Typically this is done
+using the ``MultilibBuilder`` interface. There are many examples of this in
+``Gnu.cpp``.
----------------
Where is `MultilibBuilder`?


================
Comment at: clang/docs/Multilib.rst:43
+using the ``MultilibBuilder`` interface. There are many examples of this in
+``Gnu.cpp``.
+The remainder of this document will not focus on this type of multilib.
----------------
`lib/Driver/ToolChains/Gnu.cpp`


================
Comment at: clang/docs/Multilib.rst:162
+
+  # This defines the minimum version of Clang required to use this file.
+  # It is required to be present.
----------------
Generally the comments in this example feel verbose. Consider changing some sentences with briefer but equivalent wording.

For example, "It is required to be present." is repeated several times. It can be combined with a previous sentence like "This is required and ..." or "This required key defines ..."


================
Comment at: clang/docs/Multilib.rst:229
+
+  # Example of noMatchFlags - set hasFPU if mfpu=none *doesn't* match.
+  - regex: mfpu=none
----------------
I think `Example of noMatchFlags - ` is verbose.


================
Comment at: clang/docs/Multilib.rst:237
+
+Stable API
+----------
----------------
What does "API" refer to? A function defined in llvm-project/clang/lib?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143587/new/

https://reviews.llvm.org/D143587



More information about the cfe-commits mailing list