Skip to content

Replace bool operator== for VersionType in sanitizer_mac.h #135068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 9, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions compiler-rt/lib/sanitizer_common/sanitizer_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,18 @@ struct VersionBase {

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

bool operator==(const VersionType &other) const {
return major == other.major && minor == other.minor;
}
bool operator>=(const VersionType &other) const {
return major > other.major ||
(major == other.major && minor >= other.minor);
}
bool operator<(const VersionType &other) const { return !(*this >= other); }
};

template <typename VersionType>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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); }
};

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

struct MacosVersion : VersionBase<MacosVersion> {
MacosVersion(u16 major, u16 minor) : VersionBase(major, minor) {}
};
Expand Down
Loading