[PATCH] D11862: Edits to CMake.rst for clarity's sake

[NAKAMURA Takumi via llvm-commits]

chapuni added a subscriber: chapuni.
chapuni added a comment.

LGTM, but I won't approve it since I am not native English speaker.
Thanks for you work!


================
Comment at: docs/CMake.rst:72
@@ -71,3 +71,3 @@
 
-   The underlying build tool can be invoked directly either of course, but
+   The underlying build tool can be invoked directly, of course, but
    the ``--build`` option is portable.
----------------
One nitpicking. I suppose almost all guys wouldn't use "cmake --build".
We might refine around there later. (Don't care in this review, though)

================
Comment at: docs/CMake.rst:126
@@ -119,3 +125,3 @@
 
   $ cmake -G "Visual Studio 11" path/to/llvm/source/root
 
----------------
We dropped "Visual Studio 11", aka msc17, in trunk.
We might upgrade it, regardless of this review.

================
Comment at: docs/CMake.rst:199
@@ -190,3 +198,3 @@
   Flag indicating if shared libraries will be built. Its default value is
-  OFF. Shared libraries are not supported on Windows and not recommended on the
-  other OSes.
+  OFF. Shared libraries are not supported on Windows and not recommended on
+  other operating systems.
----------------
BUILD_SHARED_LIBS is unsupported by MS toolchain, CL.EXE and LINK.EXE.

Mingw32(including mingw-w64) supports BUILD_SHARED_LIBS.

================
Comment at: docs/CMake.rst:312
@@ -301,2 +311,3 @@
 
 **LLVM_LIT_TOOLS_DIR**:PATH
+  The path to GnuWin32 tools for tests. Valid on Windows host.  Defaults to
----------------
I introduced this section.

I took %PATH%, rather than PATH or $PATH to clarify this aims Windows platform.

================
Comment at: docs/CMake.rst:330
@@ +329,3 @@
+  LLVM projects Clang, lld, and Polly, respectively, relative to the
+  top-level source directory. Defaults to ``tools/clang``, ``tools/lld``, and
+  ``tools/polly``, respectively.  If the variable for an external project
----------------
They don't have default value any more.

  - It isn't referred if in-tree subdirectory, for example llvm/tools/clang &c. exists.
  - It should be set explicitlly if corresponding subproject is out of tree.

For now, I suggest not to mention default behavior.
See also changelog in llvm/cmake/modules/AddLLVM.cmake.

================
Comment at: docs/CMake.rst:444
@@ -427,3 +443,3 @@
 
   $ make check
 
----------------
IIRC, we have a plan to let "check" obsoleted.
I suggest to use "check-llvm" or "check-all" here.


Repository:
  rL LLVM

http://reviews.llvm.org/D11862