[PATCH] D67867: [libc] Add few docs and implementation of strcpy and strcat.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 26 19:38:40 PDT 2019


MaskRay added a comment.

I get a bunch of Ninja targets. When the libc is ready, we will get thousands of targets like these:

  build strcat: phony projects/libc/src/string/strcat/strcat
  build strcat_objects: phony projects/libc/src/string/strcat/strcat_objects
  build strcat_test: phony bin/strcat_test
  build strcpy: phony projects/libc/src/string/strcpy/strcpy
  build strcpy_objects: phony projects/libc/src/string/strcpy/strcpy_objects
  build strcpy_test: phony bin/strcpy_test
  build string_h: phony projects/libc/include/string_h

Is there some way not to create the targets in the "global namespace"? The target name `bin/strcpy_test` is also less ideal. With thousands tests, those executables can "overflow" people's bin/ directories.



================
Comment at: libc/CMakeLists.txt:19
+
+set(LIBC_LINKER ld)
+
----------------
Delete this. Just use `CMAKE_LINKER`.

On Windows, the variable defaults to `set(CMAKE_LINKER "${LLVM_NATIVE_TOOLCHAIN}/bin/lld-link" CACHE FILEPATH "")`. 

compiler-rt/lib/fuzzer uses ld -r, maybe you can read its CMakeLists.txt for some insights.


================
Comment at: libc/cmake/modules/LLVMLibCRules.cmake:27
+    OUTPUT ${dest_file}
+    COMMAND cp ${src_file} ${dest_file}
+    DEPENDS ${src_file}
----------------
Use `file(COPY ...)`


================
Comment at: libc/docs/build_system.md:19
+
+Standards like POSIX require that a libc provide certain library files like
+`libc.a`, `libm.a`, etc. The targets for such library files are listed in the
----------------
I am not sure names `libm.a` and `libc.a`are required by POSIX or other standards. I think this is more of a convention. Citation needed if you state this.


================
Comment at: libc/docs/source_layout.md:70
+
+## The `testing` directory
+
----------------
This is usually named `utils/` in LLVM projects.

`test/` and `unittests/` contain lit tests and unit tests, respectively.


================
Comment at: libc/src/string/strcpy/strcpy.h:13
+#include <string.h>
+namespace __llvm_libc {
+
----------------
Blank line


================
Comment at: libc/src/string/strcpy/strcpy.h:15
+
+char *strcpy(char *dest, const char *src);
+
----------------
Delete `dest` and `src`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67867





More information about the llvm-commits mailing list