[libc-commits] [PATCH] D93195: [libc] Add memmove implementation.
Siva Chandra via Phabricator via libc-commits
libc-commits at lists.llvm.org
Tue Jan 12 14:42:45 PST 2021
sivachandra added a comment.
In D93195#2492555 <https://reviews.llvm.org/D93195#2492555>, @gchatelet wrote:
> LGTM, can you please wait for @sivachandra answer about using `assert` in code before submitting? Thx!
Sorry I missed this earlier. But, I don't see any `assert` now. Was it removed?
I don't think there is a hard and fast rule about `assert` but we should try to avoid them in order not to couple parts of the libc. For error conditions, if we align with the standards' requirements and add tests for them, I think that is sufficient.
But of course, if there is real benefit got by using an `assert`, we can use it.
================
Comment at: libc/src/string/memmove.cpp:14
+#include <stddef.h> // size_t, ptrdiff_t
+#include <unistd.h> // ssize_t
+
----------------
I think you should add a `DEPS` for this header in the CMake file?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93195/new/
https://reviews.llvm.org/D93195
More information about the libc-commits
mailing list