Guard the zero_init memcpy fallback in host builds too.

clad::zero_init falls back to zeroing an object with a byte-wise memcpy,
which is not well-defined for every type. The static_assert guarding it
was compiled only into CUDA translation units, so the same call in an
ordinary host build was unguarded. That gap is reachable from a direct
clad::zero_init call: clad's own codegen routes only arrays and VLAs
here, so it is user-written calls -- such as the one the CUDA test makes
-- that went unchecked on the host.

Compile the guard unconditionally, and skip instantiating the fill once
it has failed, so a rejected type is not memcpy'd anyway. CUDA
translation units should be unaffected, as __CUDACC__ is defined for
both their host and device passes and the assert was already active
there; that part is reasoned rather than run, since it needs a GPU, so
CI is what confirms it.

This can break a host build that previously compiled: zeroing a
non-trivially-destructible type was undefined but silent, and is now an
error. The assert also keeps its existing condition and message, both
weaker than the situation warrants -- trivial destructibility is less
than the memcpy requires, so a type with a user copy constructor and a
trivial destructor still passes, and the message still says "device
fallback" though it now fires on the host. Tightening either changes the
test that pins the message, and is left for when clad actually routes
adjoints of such types through here.

Author: vgvassilev

include/clad/Differentiator/Differentiator.h

@@ -238,34 +238,36 @@ CUDA_HOST_DEVICE auto back(TapeType& of) -> decltype(of.back()) {
   template <class T,
             typename std::enable_if<!is_range<T>::value, int>::type = 0>
   CUDA_HOST_DEVICE void zero_impl(volatile T& t) {
-#if defined(__CUDACC__)
-    static_assert(std::is_trivially_destructible<T>::value,
-                  "Clad device fallback zero_init requires trivially "
-                  "destructible types.");
-#endif
-    // Fill an array with zeros.
-    // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays)
-    unsigned char tmp[sizeof(T)] = {};
+    // Bound once so the assertion and the guard below cannot drift apart: a
+    // type the assertion rejects must not go on to be memcpy'd anyway.
+    constexpr bool is_zeroable = std::is_trivially_destructible<T>::value;
+    static_assert(is_zeroable, "Clad device fallback zero_init requires "
+                               "trivially destructible types.");
+    if constexpr (is_zeroable) {
+      // Fill an array with zeros.
+      // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays)
+      unsigned char tmp[sizeof(T)] = {};
 
 #if __has_builtin(__builtin_memcpy)
-    __builtin_memcpy(const_cast<T*>(&t), tmp, sizeof(T));
+      __builtin_memcpy(const_cast<T*>(&t), tmp, sizeof(T));
 #elif defined(__CUDACC__)
-    // Fallback for the devices that don't have __builtin_memcpy.
-    // Transfers the zero with a loop. Unlike memcpyt, this does not create the
-    // object in the destination region of storage and language semantics can't
-    // be fully preserved
-    volatile unsigned char* byte_ptr =
-        reinterpret_cast<volatile unsigned char*>(const_cast<T*>(&t));
-    for (std::size_t i = 0; i < sizeof(T); ++i)
-      byte_ptr[i] = 0;
+      // Fallback for the devices that don't have __builtin_memcpy.
+      // Transfers the zero with a loop. Unlike memcpyt, this does not create
+      // the object in the destination region of storage and language semantics
+      // can't be fully preserved
+      volatile unsigned char* byte_ptr =
+          reinterpret_cast<volatile unsigned char*>(const_cast<T*>(&t));
+      for (std::size_t i = 0; i < sizeof(T); ++i)
+        byte_ptr[i] = 0;
 #else
-    // Transfer the zeros with the magic function memcpy which can implicitly
-    // create objects in the destination region of storage immediately prior to
-    // copying the sequence of characters to the destination [27.5.1(3)].
-    // (C++ has deprecated the volatile qualifiers. However, we drop them here
-    // to make sure things still work with codebases which still have them)
-    std::memcpy(const_cast<T*>(&t), tmp, sizeof(T));
+      // Transfer the zeros with the magic function memcpy which can implicitly
+      // create objects in the destination region of storage immediately prior
+      // to copying the sequence of characters to the destination [27.5.1(3)].
+      // (C++ has deprecated the volatile qualifiers. However, we drop them here
+      // to make sure things still work with codebases which still have them)
+      std::memcpy(const_cast<T*>(&t), tmp, sizeof(T));
 #endif
+    }
   }
 
   template <class T, typename std::enable_if<is_range<T>::value, int>::type = 0>

test/Misc/ZeroInitViability.cpp

@@ -0,0 +1,67 @@
+// RUN: %cladclang %s -I%S/../../include -fsyntax-only -Xclang -verify
+//
+// Nothing is ignored: every diagnostic is spelled out below. In particular a
+// -Wnontrivial-* warning must not appear, as it would mean the rejected type
+// still reaches the memcpy, which is the thing being prevented.
+
+// clad::zero_init's fallback zeroes an object with a byte-wise memcpy, which is
+// not well-defined for every type. Its guard used to be compiled only into CUDA
+// translation units, so a direct clad::zero_init call in an ordinary host build
+// went unchecked. Check that a host build is guarded too.
+
+#include "clad/Differentiator/Differentiator.h"
+
+#include <vector>
+
+struct TrivialPod {
+  double a, b;
+};
+
+struct WithCtorTrivialCopy { // a user ctor does not stop trivial copyability
+  double a, b;
+  WithCtorTrivialCopy(double x, double y) : a(x), b(y) {}
+};
+
+struct Owning { // owns a heap handle: memcpy-zeroing it would corrupt it
+  double* p;
+  Owning() : p(new double(0)) {}
+  Owning(const Owning& o) : p(new double(*o.p)) {}
+  ~Owning() { delete p; }
+};
+
+struct HasOverload {
+  double* p;
+};
+namespace clad {
+inline void zero_init(HasOverload& h) { *h.p = 0; }
+} // namespace clad
+
+void accepted() {
+  double d = 1;
+  clad::zero_init(d);
+
+  TrivialPod p{1, 2};
+  clad::zero_init(p);
+
+  WithCtorTrivialCopy w(1, 2);
+  clad::zero_init(w);
+
+  std::vector<double> v(3);
+  clad::zero_init(v);
+
+  // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays)
+  double arr[2]{}; // a C array is iterable, and is what clad zeroes in a loop
+  clad::zero_init(arr);
+
+  HasOverload h{&d}; // an overload still wins over the fallback
+  clad::zero_init(h);
+}
+
+void rejected() {
+  Owning o;
+  // expected-error@clad/Differentiator/Differentiator.h:* {{Clad device fallback zero_init requires trivially destructible types}}
+  // expected-note@clad/Differentiator/Differentiator.h:* {{in instantiation of function template specialization 'clad::zero_impl<Owning, 0>' requested here}}
+  clad::zero_init(o); // expected-note {{in instantiation of function template specialization 'clad::zero_init<Owning>' requested here}}
+}
+
+int main() { return 0; }