[llvm][ADT] Fix compilation of headers under C++23

[avogelsgesang]

Summary

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.

A similar issue exists in ModuleSummaryIndex.h, the fix is pretty
much identical.

Fixes #59784

[avogelsgesang]

also fix ModuleSummaryIndex.h

[avogelsgesang]

not sure if I got the right audience of reviewers here. Please let me know if you think someone else would be better suited to review this

[aaron.ballman]

Thanks for the fix! LGTM, but please put NFC into the patch title so it's clear why no testing is needed for the changes.

[MaskRay]

You can remove [llvm]. llvm changes with a specific component tag doesn't need [llvm].

[avogelsgesang]

clang-format

llvm/include/llvm/ADT/APFloat.h

public:
   DoubleAPFloat(const fltSemantics &S, uninitializedTag);
   DoubleAPFloat(const fltSemantics &S, integerPart);
   DoubleAPFloat(const fltSemantics &S, const APInt &I);
   DoubleAPFloat(const fltSemantics &S, APFloat &&First, APFloat &&Second);
   DoubleAPFloat(const DoubleAPFloat &RHS);
   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);
   opStatus multiply(const DoubleAPFloat &RHS, roundingMode RM);
   opStatus divide(const DoubleAPFloat &RHS, roundingMode RM);
   opStatus remainder(const DoubleAPFloat &RHS);
   opStatus mod(const DoubleAPFloat &RHS);
   opStatus fusedMultiplyAdd(const DoubleAPFloat &Multiplicand,
if (A.isNaN())
     return A;
   if (B.isNaN())
     return B;
   if (A.isZero() && B.isZero() && (A.isNegative() != B.isNegative()))
     return A.isNegative() ? B : A;
   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
 #endif // LLVM_ADT_APFLOAT_H

llvm/include/llvm/IR/ModuleSummaryIndex.h

union NameOrGV {
     /// Summary string representation. This StringRef points to BC module
     /// string table and is valid until module data is stored in memory.
     /// This is guaranteed to happen until runThinLTOBackend function is
     /// called, so it is safe to use this field during thin link. This field
     /// is only valid if summary index was loaded from BC file.
     StringRef Name;
   } U;
 
-  GlobalValueSummaryInfo(bool HaveGVs) : U(HaveGVs) {}
+  inline GlobalValueSummaryInfo(bool HaveGVs);
 
   /// List of global value summary structures for a particular value held
   /// in the GlobalValueMap. Requires a vector in the case of multiple
   /// COMDAT values of the same name.
   GlobalValueSummaryList SummaryList;
 };
 
 /// Map from global value GUID to corresponding summary structures. Use a
public:
   /// If this is an alias summary, returns the summary of the aliased object (a
   /// global variable or function), otherwise returns itself.
   GlobalValueSummary *getBaseObject();
   const GlobalValueSummary *getBaseObject() const;
 
   friend class ModuleSummaryIndex;
 };
 
+GlobalValueSummaryInfo::GlobalValueSummaryInfo(bool HaveGVs) : U(HaveGVs) {}
+
 /// Alias summary information.
 class AliasSummary : public GlobalValueSummary {
   ValueInfo AliaseeValueInfo;
 
   /// This is the Aliasee in the same module as alias (could get from VI, trades
   /// memory for time). Note that this pointer may be null (and the value info
   /// empty) when we have a distributed index where the alias is being imported
   /// (as a copy of the aliasee), but the aliasee is not.