[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