[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