Replace bool operator== for VersionType in sanitizer_mac.h

[itf]

Fixes error: ISO C++20 considers use of overloaded operator '==' (with operand types 'MacosVersion' and 'MacosVersion') to be ambiguous despite there being a unique best viable function [-Werror,-Wambiguous-reversed-operator].

This converts the comparison operator from a non-symmetric operator (const VersionBase& (as "this") and const VersionType &). into a symmetric operator

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Ivan Tadeu Ferreira Antunes Filho (itf)

Changes

Fixes error: ISO C++20 considers use of overloaded operator '==' (with operand types 'MacosVersion' and 'MacosVersion') to be ambiguous despite there being a unique best viable function [-Werror,-Wambiguous-reversed-operator].

This converts the comparison operator from a non-symmetric operator (const VersionBase<VersionType>& (as "this") and const VersionType &). into a symmetric operator

---

Full diff: https://github.com/llvm/llvm-project/pull/135068.diff

1 Files Affected:

• (modified) compiler-rt/lib/sanitizer_common/sanitizer_mac.h (+2-2)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.h b/compiler-rt/lib/sanitizer_common/sanitizer_mac.h
index f0a97d098eea0..ebf013e8e917b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.h
@@ -37,8 +37,8 @@ struct VersionBase {
 
   VersionBase(u16 major, u16 minor) : major(major), minor(minor) {}
 
-  bool operator==(const VersionType &other) const {
-    return major == other.major && minor == other.minor;
+  friend bool operator==(const VersionType &self, const VersionType &other) {
+    return self.major == other.major && self.minor == other.minor;
   }
   bool operator>=(const VersionType &other) const {
     return major > other.major ||

[vitalybuka]

fields are public, it does not need need to be fried or even a method

[itf]

Is there a way to access VersionType without it being a method? If we didn't depend on the template for the comparison, it would be simple.

And without it being a friend, I'm unsure how to create a symmetric binary operator. Removing "friend" would cause it to have 3 parameters, and if we remove &self, we go back to the original issue we are trying to fix.

[googlewalt]

You want a standalone function, defined outside of the struct:

template <typename VersionType>
bool operator==(const VersionType &self, const VersionType &other) {
  return self.major == other.major && self.minor == other.minor;
}

[vitalybuka]

LGTM

[JDevlieghere]

This broke GreenDragon:

https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/as-lldb-cmake/23697/console

In file included from /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/compiler-rt/lib/xray/xray_profile_collector.cpp:14:
In file included from /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/compiler-rt/lib/xray/xray_profile_collector.h:20:
In file included from /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/compiler-rt/lib/xray/xray_function_call_trie.h:20:
/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/compiler-rt/lib/xray/xray_segmented_array.h:146:18: error: use of overloaded operator '==' is ambiguous (with operand types 'const Iterator<__xray::FunctionCallTrie::Node>' and 'const Iterator<__xray::FunctionCallTrie::Node>')
  146 |       return !(L == R);
      |                ~ ^  ~
/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/compiler-rt/lib/xray/xray_segmented_array.h:338:18: note: in instantiation of function template specialization '__xray::operator!=<__xray::FunctionCallTrie::Node, __xray::FunctionCallTrie::Node>' requested here
  338 |     for (auto &E : *this)
      |                  ^
/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/compiler-rt/lib/xray/xray_function_call_trie.h:325:12: note: in instantiation of member function '__xray::Array<__xray::FunctionCallTrie::Node>::~Array' requested here
  325 |   explicit FunctionCallTrie(const Allocators &A) XRAY_NEVER_INSTRUMENT
      |            ^
/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/compiler-rt/lib/xray/../sanitizer_common/sanitizer_mac.h:48:6: note: candidate function [with VersionType = __xray::Array<__xray::FunctionCallTrie::Node>::Iterator<__xray::FunctionCallTrie::Node>]
   48 | bool operator==(const VersionType &self, const VersionType &other) {
      |      ^
...

[vitalybuka]

Will this work?

struct VersionBase {
  u16 major;
  u16 minor;

  VersionBase(u16 major, u16 minor) : major(major), minor(minor) {}

  template <typename VersionType>
  bool operator==(const VersionType &other) const {
    return major == other.major && minor == other.minor;
  }

  template <typename VersionType>
  bool operator>=(const VersionType &other) const {
    return major > other.major ||
           (major == other.major && minor >= other.minor);
  }

  template <typename VersionType>
  bool operator<(const VersionType &other) const { return !(*this >= other); }
};

[vitalybuka]

I guess this will defy the purpose of two different types.

then probably:

template <typename VersionType>
bool operator==(const VersionBase<VersionType> &self, const VersionBase<VersionType> &other) {
  return self.major == other.major && self.minor == other.minor;
}

[googlewalt]

It doesn't. The code (e.g. MacosVersion immediately below) expects VersionBase to be a template. Defining an matching operator!= seems to work.

[vitalybuka]

but it's the same issue, we declare operator which can be a candidate for any type

[vitalybuka]

I see you replied to first one, which is wrong.

However this one should work. I'll try locally

template <typename VersionType>
bool operator==(const VersionBase<VersionType> &self, const VersionBase<VersionType> &other) {
  return self.major == other.major && self.minor == other.minor;
}

[googlewalt]

I see you replied to first one, which is wrong.

However this one should work. I'll try locally

template <typename VersionType>
bool operator==(const VersionBase<VersionType> &self, const VersionBase<VersionType> &other) {
  return self.major == other.major && self.minor == other.minor;
}

Yeah that seems to work.