[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 18:55:06 PDT 2019
MaskRay added inline comments.
================
Comment at: libc/cmake/modules/LLVMLibCRules.cmake:128
+ PRIVATE
+ -fpie -std=c++11
+ )
----------------
there is an existing cmake variable to specify this
llvm has moved to c++14 now.
================
Comment at: libc/cmake/modules/LLVMLibCRules.cmake:157
+ OUTPUT ${object_file}
+ DEPENDS ${object_file_raw} llvm-objcopy
+ COMMAND $<TARGET_FILE:llvm-objcopy> --add-symbol "${target_name}=.llvm.libc.entrypoint.${target_name}:0,function" ${object_file_raw} ${object_file}
----------------
Prefer CMAKE_OBJCOPY. Does this target need llvm-objcopy to build?
================
Comment at: libc/include/string.h:56
+
+void *memset(void *, int, size_t);
+
----------------
place mem* functions together
================
Comment at: libc/src/string/strcat/strcat.h:13
+#include <string.h>
+namespace __llvm_libc {
+
----------------
blank line above namespace
================
Comment at: libc/src/string/strcat/strcat.h:15
+
+char *strcat(char *dest, const char *src);
+
----------------
delete dest and src
================
Comment at: libc/src/string/strcpy/strcpy_test.cpp:18
+
+ char* result = __llvm_libc::strcpy(dest, abc.c_str());
+ ASSERT_EQ(dest, result);
----------------
clang-format
================
Comment at: libc/src/string/strcpy/strcpy_test.cpp:23
+
+ delete [] dest;
+}
----------------
clang-format.
This should be `delete[] dest`;
================
Comment at: llvm/CMakeLists.txt:62
# one for llvm+clang+... using the same sources.
-set(LLVM_ALL_PROJECTS "clang;clang-tools-extra;compiler-rt;debuginfo-tests;libclc;libcxx;libcxxabi;libunwind;lld;lldb;llgo;openmp;parallel-libs;polly;pstl")
+set(LLVM_ALL_PROJECTS "clang;clang-tools-extra;compiler-rt;debuginfo-tests;libc;libclc;libcxx;libcxxabi;libunwind;lld;lldb;llgo;openmp;parallel-libs;polly;pstl")
set(LLVM_ENABLE_PROJECTS "" CACHE STRING
----------------
Making it build by default makes me concerned: there are some cmake constructs which I am not sure build on a non-Linux platform. If nobody has tested that, probably hold off on this a bit.
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