[libc-commits] [PATCH] D77277: [libc] Fix round and memcpy to adhere to qualified calls.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

This patch largely looks good, but I have a question inline.



================
Comment at: libc/src/math/round_redirector.cpp:16
 double __round_redirector(double x) {
-  return ::round(x);
+  // Call system round since llvm-libc round is yet to be implemented.
+  return ::round(x); // LIBC-NOLINT
----------------
This comment is not quite the right thing to say. This is a redirector so the expectation is to call round from the system libc.


================
Comment at: libc/src/math/round_redirector.cpp:17
+  // Call system round since llvm-libc round is yet to be implemented.
+  return ::round(x); // LIBC-NOLINT
 }
----------------
Redirected calls should not need NOLINT annotations. Calling into system libc is correct by design. What is failing without this NOLINT annotation?


================
Comment at: libc/src/string/strcpy.cpp:18
+  return reinterpret_cast<char *>(
+      __llvm_libc::memcpy(dest, src, ::strlen(src) + 1));
 }
----------------
abrachet wrote:
> Should we wait for D77279 to use `__llvm_libc::strlen`?
Yes. Wait for D77279 and absorb the change here.


================
Comment at: libc/test/src/string/CMakeLists.txt:14
     strcpy
+    memcpy
 )
----------------
Can you add a `TODO (sivachandra)` here and below to remove the redundant deps?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77277/new/

https://reviews.llvm.org/D77277