[PATCH] D149854: [ADT] Fix compilation of `APFloat.h` under `--std=c++23`

[Adrian Vogelsgesang via Phabricator via llvm-commits]

avogelsgesang created this revision.
Herald added a project: All.
avogelsgesang requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

`DoubleAPFloat` has a `unique_ptr<APFloat[]>` member. In
`DoubleAPFloat::operator=` and `DoubleAPFloat::get{First,Second}`,
the methods of this unique_ptr are getting instantiated. At that
point `APFloat` is still only a forward declaration.

This triggers undefined behavior. So far, we were probaly just
lucky and the code compiled fine. However, with C++23
`std::unique_ptr` became constexpr, and clang (and other compilers) are
now diagnosing this latent bug as an error.

This commit fixes the issue by moving the function definitions
out of the class definition of `DoubleAPFloat`, after the declaration
of `APFloat`.

Fixes #59784


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149854

Files:
  llvm/include/llvm/ADT/APFloat.h


Index: llvm/include/llvm/ADT/APFloat.h
===================================================================
--- llvm/include/llvm/ADT/APFloat.h
+++ llvm/include/llvm/ADT/APFloat.h
@@ -635,21 +635,14 @@
   DoubleAPFloat(DoubleAPFloat &&RHS);
 
   DoubleAPFloat &operator=(const DoubleAPFloat &RHS);
-
-  DoubleAPFloat &operator=(DoubleAPFloat &&RHS) {
-    if (this != &RHS) {
-      this->~DoubleAPFloat();
-      new (this) DoubleAPFloat(std::move(RHS));
-    }
-    return *this;
-  }
+  inline DoubleAPFloat &operator=(DoubleAPFloat &&RHS);
 
   bool needsCleanup() const { return Floats != nullptr; }
 
-  APFloat &getFirst() { return Floats[0]; }
-  const APFloat &getFirst() const { return Floats[0]; }
-  APFloat &getSecond() { return Floats[1]; }
-  const APFloat &getSecond() const { return Floats[1]; }
+  inline APFloat &getFirst();
+  inline const APFloat &getFirst() const;
+  inline APFloat &getSecond();
+  inline const APFloat &getSecond() const;
 
   opStatus add(const DoubleAPFloat &RHS, roundingMode RM);
   opStatus subtract(const DoubleAPFloat &RHS, roundingMode RM);
@@ -1353,6 +1346,27 @@
   return A < B ? B : A;
 }
 
+// We want the following functions to be available in the header for inlining.
+// We cannot define them inline in the class definition of `DoubleAPFloat`
+// because doing so would instantiate `std::unique_ptr<APFloat[]>` before
+// `APFloat` is defined, and that would be undefined behavior.
+namespace detail {
+
+DoubleAPFloat & DoubleAPFloat::operator=(DoubleAPFloat &&RHS) {
+  if (this != &RHS) {
+    this->~DoubleAPFloat();
+    new (this) DoubleAPFloat(std::move(RHS));
+  }
+  return *this;
+}
+
+APFloat & DoubleAPFloat::getFirst() { return Floats[0]; }
+const APFloat & DoubleAPFloat::getFirst() const { return Floats[0]; }
+APFloat & DoubleAPFloat::getSecond() { return Floats[1]; }
+const APFloat & DoubleAPFloat::getSecond() const { return Floats[1]; }
+
+} // namespace detail
+
 } // namespace llvm
 
 #undef APFLOAT_DISPATCH_ON_SEMANTICS


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149854.519493.patch
Type: text/x-patch
Size: 2000 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230504/07ed5470/attachment.bin>