Make LLVM build in C++20 mode

brevzin · pfez · commit fac79c28a4bc · 2021-03-04T17:48:01.000+01:00
Part of the <=> changes in C++20 make certain patterns of writing equality operators ambiguous with themselves (sorry!). This patch goes through and adjusts all the comparison operators such that they should work in both C++17 and C++20 modes. It also makes two other small C++20-specific changes (adding a constructor to a type that cases to be an aggregate, and adding casts from u8 literals which no longer have type const char*). There were four categories of errors that this review fixes. Here are canonical examples of them, ordered from most to least common: // 1) Missing const namespace missing_const { struct A { #ifndef FIXED bool operator==(A const&); #else bool operator==(A const&) const; #endif }; bool a = A{} == A{}; // error } // 2) Type mismatch on CRTP namespace crtp_mismatch { template <typename Derived> struct Base { #ifndef FIXED bool operator==(Derived const&) const; #else // in one case changed to taking Base const& friend bool operator==(Derived const&, Derived const&); #endif }; struct D : Base<D> { }; bool b = D{} == D{}; // error } // 3) iterator/const_iterator with only mixed comparison namespace iter_const_iter { template <bool Const> struct iterator { using const_iterator = iterator<true>; iterator(); template <bool B, std::enable_if_t<(Const && !B), int> = 0> iterator(iterator<B> const&); #ifndef FIXED bool operator==(const_iterator const&) const; #else friend bool operator==(iterator const&, iterator const&); #endif }; bool c = iterator<false>{} == iterator<false>{} // error || iterator<false>{} == iterator<true>{} || iterator<true>{} == iterator<false>{} || iterator<true>{} == iterator<true>{}; } // 4) Same-type comparison but only have mixed-type operator namespace ambiguous_choice { enum Color { Red }; struct C { C(); C(Color); operator Color() const; bool operator==(Color) const; friend bool operator==(C, C); }; bool c = C{} == C{}; // error bool d = C{} == Red; } Differential revision: https://reviews.llvm.org/D78938
diff --git a/clang/include/clang/AST/StmtIterator.h b/clang/include/clang/AST/StmtIterator.h
@@ -104,12 +104,13 @@ class StmtIteratorImpl : public StmtIteratorBase,
     return tmp;
   }
 
-  bool operator==(const DERIVED& RHS) const {
-    return stmt == RHS.stmt && DGI == RHS.DGI && RawVAPtr == RHS.RawVAPtr;
+  friend bool operator==(const DERIVED &LHS, const DERIVED &RHS) {
+    return LHS.stmt == RHS.stmt && LHS.DGI == RHS.DGI &&
+           LHS.RawVAPtr == RHS.RawVAPtr;
   }
 
-  bool operator!=(const DERIVED& RHS) const {
-    return stmt != RHS.stmt || DGI != RHS.DGI || RawVAPtr != RHS.RawVAPtr;
+  friend bool operator!=(const DERIVED &LHS, const DERIVED &RHS) {
+    return !(LHS == RHS);
   }
 
   REFERENCE operator*() const {
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
@@ -54,6 +54,12 @@ enum OpenMPDirectiveKindEx {
 struct OpenMPDirectiveKindExWrapper {
   OpenMPDirectiveKindExWrapper(unsigned Value) : Value(Value) {}
   OpenMPDirectiveKindExWrapper(OpenMPDirectiveKind DK) : Value(unsigned(DK)) {}
+  bool operator==(OpenMPDirectiveKindExWrapper V) const {
+    return Value == V.Value;
+  }
+  bool operator!=(OpenMPDirectiveKindExWrapper V) const {
+    return Value != V.Value;
+  }
   bool operator==(OpenMPDirectiveKind V) const { return Value == unsigned(V); }
   bool operator!=(OpenMPDirectiveKind V) const { return Value != unsigned(V); }
   bool operator<(OpenMPDirectiveKind V) const { return Value < unsigned(V); }
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -101,6 +101,9 @@ class GenericTaintChecker
 
   struct FunctionData {
     FunctionData() = delete;
+    FunctionData(const FunctionDecl *FDecl, StringRef Name,
+                 std::string FullName)
+        : FDecl(FDecl), Name(Name), FullName(std::move(FullName)) {}
     FunctionData(const FunctionData &) = default;
     FunctionData(FunctionData &&) = default;
     FunctionData &operator=(const FunctionData &) = delete;
@@ -118,7 +121,7 @@ class GenericTaintChecker
       if (Name.empty() || FullName.empty())
         return None;
 
-      return FunctionData{FDecl, Name, FullName};
+      return FunctionData{FDecl, Name, std::move(FullName)};
     }
 
     bool isInScope(StringRef Scope) const {
diff --git a/llvm/include/llvm/ADT/AllocatorList.h b/llvm/include/llvm/ADT/AllocatorList.h
@@ -118,13 +118,6 @@ template <class T, class AllocatorT> class AllocatorList : AllocatorT {
 
     reference operator*() const { return base_type::wrapped()->V; }
     pointer operator->() const { return &operator*(); }
-
-    friend bool operator==(const IteratorImpl &L, const IteratorImpl &R) {
-      return L.wrapped() == R.wrapped();
-    }
-    friend bool operator!=(const IteratorImpl &L, const IteratorImpl &R) {
-      return !(L == R);
-    }
   };
 
 public:
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
@@ -1211,22 +1211,18 @@ class DenseMapIterator : DebugEpochBase::HandleBase {
     return Ptr;
   }
 
-  template <typename T>
-  bool operator==(const T &RHS) const {
-    assert((!Ptr || isHandleInSync()) && "handle not in sync!");
+  friend bool operator==(const DenseMapIterator &LHS,
+                         const DenseMapIterator &RHS) {
+    assert((!LHS.Ptr || LHS.isHandleInSync()) && "handle not in sync!");
     assert((!RHS.Ptr || RHS.isHandleInSync()) && "handle not in sync!");
-    assert(getEpochAddress() == RHS.getEpochAddress() &&
+    assert(LHS.getEpochAddress() == RHS.getEpochAddress() &&
            "comparing incomparable iterators!");
-    return Ptr == RHS.Ptr;
+    return LHS.Ptr == RHS.Ptr;
   }
 
-  template <typename T>
-  bool operator!=(const T &RHS) const {
-    assert((!Ptr || isHandleInSync()) && "handle not in sync!");
-    assert((!RHS.Ptr || RHS.isHandleInSync()) && "handle not in sync!");
-    assert(getEpochAddress() == RHS.getEpochAddress() &&
-           "comparing incomparable iterators!");
-    return Ptr != RHS.Ptr;
+  friend bool operator!=(const DenseMapIterator &LHS,
+                         const DenseMapIterator &RHS) {
+    return !(LHS == RHS);
   }
 
   inline DenseMapIterator& operator++() {  // Preincrement
diff --git a/llvm/include/llvm/ADT/DenseSet.h b/llvm/include/llvm/ADT/DenseSet.h
@@ -124,8 +124,12 @@ class DenseSetImpl {
 
     Iterator& operator++() { ++I; return *this; }
     Iterator operator++(int) { auto T = *this; ++I; return T; }
-    bool operator==(const ConstIterator& X) const { return I == X.I; }
-    bool operator!=(const ConstIterator& X) const { return I != X.I; }
+    friend bool operator==(const Iterator &X, const Iterator &Y) {
+      return X.I == Y.I;
+    }
+    friend bool operator!=(const Iterator &X, const Iterator &Y) {
+      return X.I != Y.I;
+    }
   };
 
   class ConstIterator {
@@ -149,8 +153,12 @@ class DenseSetImpl {
 
     ConstIterator& operator++() { ++I; return *this; }
     ConstIterator operator++(int) { auto T = *this; ++I; return T; }
-    bool operator==(const ConstIterator& X) const { return I == X.I; }
-    bool operator!=(const ConstIterator& X) const { return I != X.I; }
+    friend bool operator==(const ConstIterator &X, const ConstIterator &Y) {
+      return X.I == Y.I;
+    }
+    friend bool operator!=(const ConstIterator &X, const ConstIterator &Y) {
+      return X.I != Y.I;
+    }
   };
 
   using iterator = Iterator;
diff --git a/llvm/include/llvm/ADT/DirectedGraph.h b/llvm/include/llvm/ADT/DirectedGraph.h
@@ -38,8 +38,10 @@ template <class NodeType, class EdgeType> class DGEdge {
 
   /// Static polymorphism: delegate implementation (via isEqualTo) to the
   /// derived class.
-  bool operator==(const EdgeType &E) const { return getDerived().isEqualTo(E); }
-  bool operator!=(const EdgeType &E) const { return !operator==(E); }
+  bool operator==(const DGEdge &E) const {
+    return getDerived().isEqualTo(E.getDerived());
+  }
+  bool operator!=(const DGEdge &E) const { return !operator==(E); }
 
   /// Retrieve the target node this edge connects to.
   const NodeType &getTargetNode() const { return TargetNode; }
@@ -91,8 +93,12 @@ template <class NodeType, class EdgeType> class DGNode {
 
   /// Static polymorphism: delegate implementation (via isEqualTo) to the
   /// derived class.
-  bool operator==(const NodeType &N) const { return getDerived().isEqualTo(N); }
-  bool operator!=(const NodeType &N) const { return !operator==(N); }
+  friend bool operator==(const NodeType &M, const NodeType &N) {
+    return M.isEqualTo(N);
+  }
+  friend bool operator!=(const NodeType &M, const NodeType &N) {
+    return !(M == N);
+  }
 
   const_iterator begin() const { return Edges.begin(); }
   const_iterator end() const { return Edges.end(); }
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
@@ -486,12 +486,12 @@ class early_inc_iterator_impl
     return *this;
   }
 
-  using BaseT::operator==;
-  bool operator==(const early_inc_iterator_impl &RHS) const {
+  friend bool operator==(const early_inc_iterator_impl &LHS,
+                         const early_inc_iterator_impl &RHS) {
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS
-    assert(!IsEarlyIncremented && "Cannot compare after dereferencing!");
+    assert(!LHS.IsEarlyIncremented && "Cannot compare after dereferencing!");
 #endif
-    return BaseT::operator==(RHS);
+    return (const BaseT &)LHS == (const BaseT &)RHS;
   }
 };
 
diff --git a/llvm/include/llvm/ADT/StringMap.h b/llvm/include/llvm/ADT/StringMap.h
@@ -505,7 +505,9 @@ class StringMapIterBase
     return static_cast<DerivedTy &>(*this);
   }
 
-  bool operator==(const DerivedTy &RHS) const { return Ptr == RHS.Ptr; }
+  friend bool operator==(const DerivedTy &LHS, const DerivedTy &RHS) {
+    return LHS.Ptr == RHS.Ptr;
+  }
 
   DerivedTy &operator++() { // Preincrement
     ++Ptr;
diff --git a/llvm/include/llvm/ADT/iterator.h b/llvm/include/llvm/ADT/iterator.h
@@ -142,28 +142,30 @@ class iterator_facade_base
     return tmp;
   }
 
+#ifndef __cpp_impl_three_way_comparison
   bool operator!=(const DerivedT &RHS) const {
-    return !static_cast<const DerivedT *>(this)->operator==(RHS);
+    return !(static_cast<const DerivedT &>(*this) == RHS);
   }
+#endif
 
   bool operator>(const DerivedT &RHS) const {
     static_assert(
         IsRandomAccess,
         "Relational operators are only defined for random access iterators.");
-    return !static_cast<const DerivedT *>(this)->operator<(RHS) &&
-           !static_cast<const DerivedT *>(this)->operator==(RHS);
+    return !(static_cast<const DerivedT &>(*this) < RHS) &&
+           !(static_cast<const DerivedT &>(*this) == RHS);
   }
   bool operator<=(const DerivedT &RHS) const {
     static_assert(
         IsRandomAccess,
         "Relational operators are only defined for random access iterators.");
-    return !static_cast<const DerivedT *>(this)->operator>(RHS);
+    return !(static_cast<const DerivedT &>(*this) > RHS);
   }
   bool operator>=(const DerivedT &RHS) const {
     static_assert(
         IsRandomAccess,
         "Relational operators are only defined for random access iterators.");
-    return !static_cast<const DerivedT *>(this)->operator<(RHS);
+    return !(static_cast<const DerivedT &>(*this) < RHS);
   }
 
   PointerT operator->() { return &static_cast<DerivedT *>(this)->operator*(); }
@@ -260,12 +262,16 @@ class iterator_adaptor_base
     return *static_cast<DerivedT *>(this);
   }
 
-  bool operator==(const DerivedT &RHS) const { return I == RHS.I; }
-  bool operator<(const DerivedT &RHS) const {
+  friend bool operator==(const iterator_adaptor_base &LHS,
+                         const iterator_adaptor_base &RHS) {
+    return LHS.I == RHS.I;
+  }
+  friend bool operator<(const iterator_adaptor_base &LHS,
+                        const iterator_adaptor_base &RHS) {
     static_assert(
         BaseT::IsRandomAccess,
         "Relational operators are only defined for random access iterators.");
-    return I < RHS.I;
+    return LHS.I < RHS.I;
   }
 
   ReferenceT operator*() const { return *I; }
diff --git a/llvm/include/llvm/CodeGen/DIE.h b/llvm/include/llvm/CodeGen/DIE.h
@@ -578,7 +578,6 @@ template <class T> class IntrusiveBackList : IntrusiveBackListBase {
     T &operator*() const { return *static_cast<T *>(N); }
 
     bool operator==(const iterator &X) const { return N == X.N; }
-    bool operator!=(const iterator &X) const { return N != X.N; }
   };
 
   class const_iterator
@@ -601,7 +600,6 @@ template <class T> class IntrusiveBackList : IntrusiveBackListBase {
     const T &operator*() const { return *static_cast<const T *>(N); }
 
     bool operator==(const const_iterator &X) const { return N == X.N; }
-    bool operator!=(const const_iterator &X) const { return N != X.N; }
   };
 
   iterator begin() {
diff --git a/llvm/include/llvm/CodeGen/LiveInterval.h b/llvm/include/llvm/CodeGen/LiveInterval.h
@@ -730,10 +730,10 @@ namespace llvm {
         ++*this;
         return res;
       }
-      bool operator!=(const SingleLinkedListIterator<T> &Other) {
+      bool operator!=(const SingleLinkedListIterator<T> &Other) const {
         return P != Other.operator->();
       }
-      bool operator==(const SingleLinkedListIterator<T> &Other) {
+      bool operator==(const SingleLinkedListIterator<T> &Other) const {
         return P == Other.operator->();
       }
       T &operator*() const {
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
@@ -369,11 +369,6 @@ inline bool operator==(const DWARFDie::iterator &LHS,
   return LHS.Die == RHS.Die;
 }
 
-inline bool operator!=(const DWARFDie::iterator &LHS,
-                       const DWARFDie::iterator &RHS) {
-  return !(LHS == RHS);
-}
-
 // These inline functions must follow the DWARFDie::iterator definition above
 // as they use functions from that class.
 inline DWARFDie::iterator DWARFDie::begin() const {
@@ -455,11 +450,6 @@ inline bool operator==(const std::reverse_iterator<DWARFDie::iterator> &LHS,
   return LHS.equals(RHS);
 }
 
-inline bool operator!=(const std::reverse_iterator<DWARFDie::iterator> &LHS,
-                       const std::reverse_iterator<DWARFDie::iterator> &RHS) {
-  return !(LHS == RHS);
-}
-
 inline std::reverse_iterator<DWARFDie::iterator> DWARFDie::rbegin() const {
   return llvm::make_reverse_iterator(end());
 }
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
@@ -150,10 +150,5 @@ inline bool operator==(const DWARFExpression::iterator &LHS,
                        const DWARFExpression::iterator &RHS) {
   return LHS.Expr == RHS.Expr && LHS.Offset == RHS.Offset;
 }
-
-inline bool operator!=(const DWARFExpression::iterator &LHS,
-                       const DWARFExpression::iterator &RHS) {
-  return !(LHS == RHS);
-}
 }
 #endif
diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h
@@ -862,10 +862,8 @@ class AttrBuilder {
 
   bool td_empty() const { return TargetDepAttrs.empty(); }
 
-  bool operator==(const AttrBuilder &B);
-  bool operator!=(const AttrBuilder &B) {
-    return !(*this == B);
-  }
+  bool operator==(const AttrBuilder &B) const;
+  bool operator!=(const AttrBuilder &B) const { return !(*this == B); }
 };
 
 namespace AttributeFuncs {
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
@@ -304,7 +304,9 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
     phi_iterator_impl() = default;
 
     // Allow conversion between instantiations where valid.
-    template <typename PHINodeU, typename BBIteratorU>
+    template <typename PHINodeU, typename BBIteratorU,
+              typename = std::enable_if_t<
+                  std::is_convertible<PHINodeU *, PHINodeT *>::value>>
     phi_iterator_impl(const phi_iterator_impl<PHINodeU, BBIteratorU> &Arg)
         : PN(Arg.PN) {}
 
diff --git a/llvm/include/llvm/Object/StackMapParser.h b/llvm/include/llvm/Object/StackMapParser.h
@@ -35,11 +35,13 @@ class StackMapParser {
       return tmp;
     }
 
-    bool operator==(const AccessorIterator &Other) {
+    bool operator==(const AccessorIterator &Other) const {
       return A.P == Other.A.P;
     }
 
-    bool operator!=(const AccessorIterator &Other) { return !(*this == Other); }
+    bool operator!=(const AccessorIterator &Other) const {
+      return !(*this == Other);
+    }
 
     AccessorT& operator*() { return A; }
     AccessorT* operator->() { return &A; }
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
@@ -67,10 +67,10 @@ class CoverageMappingIterator
     increment();
     return *this;
   }
-  bool operator==(const CoverageMappingIterator &RHS) {
+  bool operator==(const CoverageMappingIterator &RHS) const {
     return Reader == RHS.Reader;
   }
-  bool operator!=(const CoverageMappingIterator &RHS) {
+  bool operator!=(const CoverageMappingIterator &RHS) const {
     return Reader != RHS.Reader;
   }
   Expected<CoverageMappingRecord &> operator*() {
diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -50,8 +50,12 @@ class InstrProfIterator : public std::iterator<std::input_iterator_tag,
   InstrProfIterator(InstrProfReader *Reader) : Reader(Reader) { Increment(); }
 
   InstrProfIterator &operator++() { Increment(); return *this; }
-  bool operator==(const InstrProfIterator &RHS) { return Reader == RHS.Reader; }
-  bool operator!=(const InstrProfIterator &RHS) { return Reader != RHS.Reader; }
+  bool operator==(const InstrProfIterator &RHS) const {
+    return Reader == RHS.Reader;
+  }
+  bool operator!=(const InstrProfIterator &RHS) const {
+    return Reader != RHS.Reader;
+  }
   value_type &operator*() { return Record; }
   value_type *operator->() { return &Record; }
 };
diff --git a/llvm/include/llvm/Support/BinaryStreamRef.h b/llvm/include/llvm/Support/BinaryStreamRef.h
@@ -121,12 +121,12 @@ template <class RefType, class StreamType> class BinaryStreamRefBase {
 
   bool valid() const { return BorrowedImpl != nullptr; }
 
-  bool operator==(const RefType &Other) const {
-    if (BorrowedImpl != Other.BorrowedImpl)
+  friend bool operator==(const RefType &LHS, const RefType &RHS) {
+    if (LHS.BorrowedImpl != RHS.BorrowedImpl)
       return false;
-    if (ViewOffset != Other.ViewOffset)
+    if (LHS.ViewOffset != RHS.ViewOffset)
       return false;
-    if (Length != Other.Length)
+    if (LHS.Length != RHS.Length)
       return false;
     return true;
   }
diff --git a/llvm/lib/CodeGen/PeepholeOptimizer.cpp b/llvm/lib/CodeGen/PeepholeOptimizer.cpp
@@ -330,7 +330,7 @@ namespace {
       return RegSrcs[Idx].SubReg;
     }
 
-    bool operator==(const ValueTrackerResult &Other) {
+    bool operator==(const ValueTrackerResult &Other) const {
       if (Other.getInst() != getInst())
         return false;
 
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
diff --git a/llvm/lib/Transforms/Scalar/GVNHoist.cpp b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
diff --git a/llvm/unittests/ADT/STLExtrasTest.cpp b/llvm/unittests/ADT/STLExtrasTest.cpp

Original file line number	Diff line number	Diff line change
`@@ -104,12 +104,13 @@ class StmtIteratorImpl : public StmtIteratorBase,`
`104`	`104`	`return tmp;`
`105`	`105`	`}`
`106`	`106`
`107`		`- bool operator==(const DERIVED& RHS) const {`
`108`		`- return stmt == RHS.stmt && DGI == RHS.DGI && RawVAPtr == RHS.RawVAPtr;`
	`107`	`+ friend bool operator==(const DERIVED &LHS, const DERIVED &RHS) {`
	`108`	`+ return LHS.stmt == RHS.stmt && LHS.DGI == RHS.DGI &&`
	`109`	`+ LHS.RawVAPtr == RHS.RawVAPtr;`
`109`	`110`	`}`
`110`	`111`
`111`		`- bool operator!=(const DERIVED& RHS) const {`
`112`		`- return stmt != RHS.stmt \|\| DGI != RHS.DGI \|\| RawVAPtr != RHS.RawVAPtr;`
	`112`	`+ friend bool operator!=(const DERIVED &LHS, const DERIVED &RHS) {`
	`113`	`+ return !(LHS == RHS);`
`113`	`114`	`}`
`114`	`115`
`115`	`116`	`REFERENCE operator*() const {`
Original file line number	Diff line number	Diff line change
`@@ -505,7 +505,9 @@ class StringMapIterBase`
`505`	`505`	`return static_cast<DerivedTy &>(*this);`
`506`	`506`	`}`
`507`	`507`
`508`		`- bool operator==(const DerivedTy &RHS) const { return Ptr == RHS.Ptr; }`
	`508`	`+ friend bool operator==(const DerivedTy &LHS, const DerivedTy &RHS) {`
	`509`	`+ return LHS.Ptr == RHS.Ptr;`
	`510`	`+ }`
`509`	`511`
`510`	`512`	`DerivedTy &operator++() { // Preincrement`
`511`	`513`	`++Ptr;`