[libcxx-commits] [PATCH] D120684: [libc++] Define _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER whenever we enable warnings in the test suite

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Mar 3 15:24:09 PST 2022


Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__algorithm/copy.h:56
     if (__n > 0)
-        _VSTD::memmove(__result, __first, __n * sizeof(_Up));
+        _VSTD::memmove(static_cast<void*>(__result), static_cast<void const*>(__first), __n * sizeof(_Up));
     return __result + __n;
----------------
Here and throughout: what is the compiler complaining about if you don't do this? Implicit conversion from `T*` to `void*` is absolutely part of C++ since forever, until forever; and this isn't an ADL situation either; so I can't imagine what a compiler could possibly see to warn about here.


================
Comment at: libcxx/include/any:367
         }
+        __libcpp_unreachable();
     }
----------------
`#include <__utility/unreachable.h>`, right?


================
Comment at: libcxx/include/experimental/simd:1473-1474
 
+  _LIBCPP_DIAGNOSTIC_PUSH
+  _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wnon-template-friend")
   // binary operators [simd.binary]
----------------
Just from a quick glance at this code, I'm confused. Where are the definitions of these friend functions? If they don't actually exist, then perhaps these lines should just be removed, or at least `#if 0`'d out.


================
Comment at: libcxx/include/latch:94
     {
-        auto const __test_fn = [=]() -> bool {
+        auto const __test_fn = [this]() -> bool {
             return try_wait();
----------------
Let's use `[&]` here, because that's the "default, 99% of the time" thing. We're not doing anything special here that would justify deviating from the default thing.
Optionally, inline the lambda in the one place it's used:
```
__cxx_atomic_wait(&__a.__a_, [&]() {
    return try_wait();
});
```


================
Comment at: libcxx/test/std/concepts/concepts.lang/concept.default.init/default_initializable.compile.pass.cpp:12-14
+// We voluntarily use std::default_initializable on types that have redundant
+// or ignored cv-qualifiers -- don't warn about it.
+// ADDITIONAL_COMPILE_FLAGS: -Wno-ignored-qualifiers
----------------
I don't understand this comment.


================
Comment at: libcxx/test/support/type_classification/movable.h:71-72
+  copy_with_mutable_parameter(copy_with_mutable_parameter&);
+  copy_with_mutable_parameter&
+  operator=(copy_with_mutable_parameter&);
 };
----------------
I don't understand the relevance of these changes, but I certainly don't object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120684



More information about the libcxx-commits mailing list