Skip to content
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

Vector Equals Matcher uses != for comparisons #2648

Closed
js-nano opened this issue Feb 22, 2023 · 2 comments
Closed

Vector Equals Matcher uses != for comparisons #2648

js-nano opened this issue Feb 22, 2023 · 2 comments

Comments

@js-nano
Copy link

js-nano commented Feb 22, 2023

The vector Equals matcher uses != to compare elements:

if (m_comparator[i] != v[i])

I'd expect it to use == to compare elements (as implied by the Matcher's name 🙂)

Impacts are:

  • Prior to C++20, an operator!= needs to be supplied for user types (in addition to operator==).
    • If you are using C++20, the situation is happier as your compiler can rewrite != to use operator== (provided your compiler supports P1185R2)
  • If you're a monster and your operator== and operator!= are not the inverse of each other, the "wrong" operator would unexpectedly be used (again, based on the Matcher's name).

NB - In addition, != is used for approx matching here, though I don't think that would cause any of the issues noted above?

if (m_comparator[i] != approx(v[i]))

@js-nano js-nano changed the title Vector Equals Matcher uses operator!= Vector Equals Matcher uses != for comparisons Feb 22, 2023
@horenmar
Copy link
Member

horenmar commented Mar 4, 2023

I'll change this, but I have to say that in this case

If you're a monster and your operator== and operator!= are not the inverse of each other

it is the user's fault and I don't care.

@js-nano
Copy link
Author

js-nano commented Mar 6, 2023

Thanks for this!

As regards

it is the user's fault and I don't care.

Yes, I agree completely!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants