mstorsjo added inline comments.

================
Comment at: cmake/Modules/HandleCompilerRT.cmake:17
   string(STRIP "${LIBRARY_FILE}" LIBRARY_FILE)
+  file(TO_CMAKE_PATH ${LIBRARY_FILE} LIBRARY_FILE)
   string(REPLACE "builtins" "${name}" LIBRARY_FILE "${LIBRARY_FILE}")
----------------
smeenai wrote:
> Nit: cmake recommends always putting quotes around the path argument to 
> ensure it's treated as a single argument, i.e.
> 
>   file(TO_CMAKE_PATH "${LIBRARY_FILE}" LIBRARY_FILE)
> 
> That needs to happen in all other uses as well.
Ok, will do.


================
Comment at: cmake/Modules/HandleOutOfTreeLLVM.cmake:52
     else()
-      set(LLVM_CMAKE_PATH
-          "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+      file(TO_CMAKE_PATH ${LLVM_BINARY_DIR} LLVM_BINARY_DIR_CMAKE_STYLE)
+      set(LLVM_CMAKE_PATH 
"${LLVM_BINARY_DIR_CMAKE_STYLE}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
----------------
smeenai wrote:
> Is there ever a chance LLVM_BINARY_DIR will have backslashes, or are you just 
> being extra careful (or going for consistency)?
I'm mostly just going for consistency and copy+paste; this path identically 
matches a part from compiler-rt/cmake/Modules/CompilerRTUtils.cmake - and that 
part actually seems to have been added intentionally to fix some case: 
https://reviews.llvm.org/rL203789


Repository:
  rCXX libc++

https://reviews.llvm.org/D48356



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to