Просмотр исходного кода

Make binary operators non-member (#4838)

Came up on #4831, style:

"For a type T whose values can be compared for equality, define a
non-member operator== and document when two values of type T are
considered equal."
https://google.github.io/styleguide/cppguide.html#Operator_Overloading

Note while we could put some of these out-of-line, it's helpful to keep
them inside the braces:
- For private member access
- For templated cases so that we aren't duplicating templates
- Very mild preference for keeping class's API documented within the
braces
Jon Ross-Perkins 1 год назад
Родитель
Сommit
1670baf180

+ 2 - 2
common/raw_hashtable_metadata_group.h

@@ -192,8 +192,8 @@ class BitIndexRange
     Iterator() = default;
     explicit Iterator(BitsT bits) : bits_(bits) {}
 
-    auto operator==(const Iterator& rhs) const -> bool {
-      return bits_ == rhs.bits_;
+    friend auto operator==(const Iterator& lhs, const Iterator& rhs) -> bool {
+      return lhs.bits_ == rhs.bits_;
     }
 
     auto operator*() -> ssize_t& {

+ 14 - 10
toolchain/base/index_base.h

@@ -58,8 +58,8 @@ struct IdBase : public AnyIdBase, public Printable<IdT> {
   }
 
   // Support simple equality comparison for ID types.
-  constexpr auto operator==(IdBase<IdT> rhs) const -> bool {
-    return index == rhs.index;
+  friend constexpr auto operator==(IdBase<IdT> lhs, IdBase<IdT> rhs) -> bool {
+    return lhs.index == rhs.index;
   }
 };
 
@@ -73,8 +73,9 @@ struct IndexBase : public IdBase<IdT> {
   using IdBase<IdT>::IdBase;
 
   // Support relational comparisons for index types.
-  auto operator<=>(IndexBase<IdT> rhs) const -> std::strong_ordering {
-    return this->index <=> rhs.index;
+  friend auto operator<=>(IndexBase<IdT> lhs, IndexBase<IdT> rhs)
+      -> std::strong_ordering {
+    return lhs.index <=> rhs.index;
   }
 };
 
@@ -90,11 +91,13 @@ class IndexIterator
 
   explicit IndexIterator(IndexT index) : index_(index) {}
 
-  auto operator==(const IndexIterator& rhs) const -> bool {
-    return index_ == rhs.index_;
+  friend auto operator==(const IndexIterator& lhs, const IndexIterator& rhs)
+      -> bool {
+    return lhs.index_ == rhs.index_;
   }
-  auto operator<=>(const IndexIterator& rhs) const -> std::strong_ordering {
-    return index_ <=> rhs.index_;
+  friend auto operator<=>(const IndexIterator& lhs, const IndexIterator& rhs)
+      -> std::strong_ordering {
+    return lhs.index_ <=> rhs.index_;
   }
 
   auto operator*() const -> const IndexT& { return index_; }
@@ -102,8 +105,9 @@ class IndexIterator
   using llvm::iterator_facade_base<IndexIterator,
                                    std::random_access_iterator_tag,
                                    const IndexT, int>::operator-;
-  auto operator-(const IndexIterator& rhs) const -> int {
-    return index_.index - rhs.index_.index;
+  friend auto operator-(const IndexIterator& lhs, const IndexIterator& rhs)
+      -> int {
+    return lhs.index_.index - rhs.index_.index;
   }
 
   auto operator+=(int n) -> IndexIterator& {

+ 3 - 2
toolchain/parse/node_kind.h

@@ -54,8 +54,9 @@ class NodeCategory : public Printable<NodeCategory> {
   // Returns the set inverse.
   constexpr auto operator~() -> NodeCategory { return ~value_; }
 
-  auto operator==(const NodeCategory& other) const -> bool {
-    return value_ == other.value_;
+  friend auto operator==(const NodeCategory& lhs, const NodeCategory& rhs)
+      -> bool {
+    return lhs.value_ == rhs.value_;
   }
 
   auto Print(llvm::raw_ostream& out) const -> void;

+ 10 - 6
toolchain/parse/tree.h

@@ -289,21 +289,25 @@ class Tree::PostorderIterator
 
   PostorderIterator() = delete;
 
-  auto operator==(const PostorderIterator& rhs) const -> bool {
-    return node_ == rhs.node_;
+  friend auto operator==(const PostorderIterator& lhs,
+                         const PostorderIterator& rhs) -> bool {
+    return lhs.node_ == rhs.node_;
   }
   // While we don't want users to directly leverage the index of `NodeId` for
   // ordering, when we're explicitly walking in postorder, that becomes
   // reasonable so add the ordering here and reach down for the index
   // explicitly.
-  auto operator<=>(const PostorderIterator& rhs) const -> std::strong_ordering {
-    return node_.index <=> rhs.node_.index;
+  friend auto operator<=>(const PostorderIterator& lhs,
+                          const PostorderIterator& rhs)
+      -> std::strong_ordering {
+    return lhs.node_.index <=> rhs.node_.index;
   }
 
   auto operator*() const -> NodeId { return node_; }
 
-  auto operator-(const PostorderIterator& rhs) const -> int {
-    return node_.index - rhs.node_.index;
+  friend auto operator-(const PostorderIterator& lhs,
+                        const PostorderIterator& rhs) -> int {
+    return lhs.node_.index - rhs.node_.index;
   }
 
   auto operator+=(int offset) -> PostorderIterator& {

+ 3 - 2
toolchain/parse/tree_and_subtrees.h

@@ -197,8 +197,9 @@ class TreeAndSubtrees::SiblingIterator
  public:
   explicit SiblingIterator() = delete;
 
-  auto operator==(const SiblingIterator& rhs) const -> bool {
-    return node_ == rhs.node_;
+  friend auto operator==(const SiblingIterator& lhs, const SiblingIterator& rhs)
+      -> bool {
+    return lhs.node_ == rhs.node_;
   }
 
   auto operator*() const -> NodeId { return node_; }

+ 20 - 13
toolchain/sem_ir/facet_type_info.h

@@ -28,12 +28,16 @@ struct FacetTypeInfo : Printable<FacetTypeInfo> {
     InterfaceId interface_id;
     SpecificId specific_id;
 
-    auto operator==(const ImplsConstraint& rhs) const -> bool {
-      return interface_id == rhs.interface_id && specific_id == rhs.specific_id;
+    friend auto operator==(const ImplsConstraint& lhs,
+                           const ImplsConstraint& rhs) -> bool {
+      return lhs.interface_id == rhs.interface_id &&
+             lhs.specific_id == rhs.specific_id;
     }
     // Canonically ordered by the numerical ids.
-    auto operator<=>(const ImplsConstraint& rhs) const -> std::strong_ordering {
-      return std::tie(interface_id.index, specific_id.index) <=>
+    friend auto operator<=>(const ImplsConstraint& lhs,
+                            const ImplsConstraint& rhs)
+        -> std::strong_ordering {
+      return std::tie(lhs.interface_id.index, lhs.specific_id.index) <=>
              std::tie(rhs.interface_id.index, rhs.specific_id.index);
     }
   };
@@ -44,14 +48,16 @@ struct FacetTypeInfo : Printable<FacetTypeInfo> {
     ConstantId lhs_const_id;
     ConstantId rhs_const_id;
 
-    auto operator==(const RewriteConstraint& rhs) const -> bool {
-      return lhs_const_id == rhs.lhs_const_id &&
-             rhs_const_id == rhs.rhs_const_id;
+    friend auto operator==(const RewriteConstraint& lhs,
+                           const RewriteConstraint& rhs) -> bool {
+      return lhs.lhs_const_id == rhs.lhs_const_id &&
+             lhs.rhs_const_id == rhs.rhs_const_id;
     }
     // Canonically ordered by the numerical ids.
-    auto operator<=>(const RewriteConstraint& rhs) const
+    friend auto operator<=>(const RewriteConstraint& lhs,
+                            const RewriteConstraint& rhs)
         -> std::strong_ordering {
-      return std::tie(lhs_const_id.index, rhs_const_id.index) <=>
+      return std::tie(lhs.lhs_const_id.index, lhs.rhs_const_id.index) <=>
              std::tie(rhs.lhs_const_id.index, rhs.rhs_const_id.index);
     }
   };
@@ -78,10 +84,11 @@ struct FacetTypeInfo : Printable<FacetTypeInfo> {
     return std::nullopt;
   }
 
-  auto operator==(const FacetTypeInfo& rhs) const -> bool {
-    return impls_constraints == rhs.impls_constraints &&
-           rewrite_constraints == rhs.rewrite_constraints &&
-           other_requirements == rhs.other_requirements;
+  friend auto operator==(const FacetTypeInfo& lhs, const FacetTypeInfo& rhs)
+      -> bool {
+    return lhs.impls_constraints == rhs.impls_constraints &&
+           lhs.rewrite_constraints == rhs.rewrite_constraints &&
+           lhs.other_requirements == rhs.other_requirements;
   }
 };
 

+ 3 - 2
toolchain/sem_ir/import_ir.h

@@ -35,8 +35,9 @@ struct ImportIRInst : public Printable<ImportIRInst> {
     out << "{ir_id: " << ir_id << ", inst_id: " << inst_id << "}";
   }
 
-  auto operator==(const ImportIRInst& rhs) const -> bool {
-    return ir_id == rhs.ir_id && inst_id == rhs.inst_id;
+  friend auto operator==(const ImportIRInst& lhs, const ImportIRInst& rhs)
+      -> bool {
+    return lhs.ir_id == rhs.ir_id && lhs.inst_id == rhs.inst_id;
   }
 
   ImportIRId ir_id;