[PATCH] D67867: [libc] Add few docs and implementation of strcpy and strcat.
David Chisnall via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 27 03:13:09 PDT 2019
theraven added inline comments.
================
Comment at: libc/include/math.h:20
+
+long double acosl(long double);
+
----------------
The `long long` functions should be exposed only for C99 or later and a version of C++ that supports the `long long` type.
================
Comment at: libc/include/string.h:12
+
+#include <stddef.h> // For size_t
+
----------------
Namespace pollution. The standard expects `size_t` to be exposed by this header, but not the other types in `stddef.h`. Software that relies on this pollution is non-portable (and will break on existing libc implementations that follow the standard).
================
Comment at: libc/include/string.h:18
+
+void *memcpy(void *, const void *, size_t);
+
----------------
Missing `restrict` qualifiers on this and many other standard functions in this file. The standard defines `memcpy` as:
```
void *memcpy(void * restrict s1,
const void * restrict s2,
size_t n);
```
I'm surprised that clang doesn't warn about the declaration of `memcpy` with incorrect types - it usually notices missing qualifiers.
================
Comment at: libc/src/string/strcpy/strcpy_test.cpp:16
+ std::string abc = "abc";
+ char *dest = new char[4];
+
----------------
Having the libc test suite depend on a working C++ runtime and standard library is likely to make unit testing the library difficult. The implementation of `std::string` almost certainly depends on several `libc` functions and `new` and `delete` probably do as well. This means that we can test the namespaced libc functions in an environment that already has a libc, but we can't easily extend these tests to run in a process (or other environment) that has only LLVM libc.
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