15

Here is the MWE:

#include <iostream>
#include <tuple>
#include <queue>
using namespace std;

bool operator<(tuple<int, int, int> lhs, tuple<int, int, int> rhs)
{
    return get<1>(lhs) < get<1>(rhs);
}

int main()
{
    priority_queue<tuple<int, int, int>> q;
    q.push(make_tuple(2, 5, 3));
    q.push(make_tuple(2, 3, 3));
    cout << get<1>(q.top());
    return 0;
}

The weird part is that whether I type < or > in the sentence return get<1>(lhs) < get<1>(rhs);, the output is always 5. Why does this happen?

user2357112
  • 260,549
  • 28
  • 431
  • 505
MKCCT
  • 177
  • 6
  • 6
    My best guess: it's using the default comparison operator for generic tuples. Probably because your operator is not in the std namespace. Try implementing a custom comparator that explicitly calls your operator. – Wutz Aug 16 '22 at 07:03
  • 3
    See dupe: [Why does `std::priority_queue` return largest elements first](https://stackoverflow.com/questions/44750632/why-does-stdpriority-queue-return-largest-elements-first-but-use-stdless) – Jason Aug 16 '22 at 07:04
  • 7
    @JasonLiam it's not a dupe of that – Wutz Aug 16 '22 at 07:08
  • 7
    You should probably not use `std::tuple` for things like this, since it comes with it's own comparison operator overloads. You should probably define your own `struct` with three `int`s... That way you can also give your members proper names. But if you really want this, you can specify the compare function for the `priority_queue`, e.g. `auto cmp = [](tuple lhs, tuple rhs) { return get<1>(lhs) < get<1>(rhs); };` and `priority_queue,std::vector>, decltype(cmp)> q;` – JHBonarius Aug 16 '22 at 07:24

2 Answers2

23

Your overload of operator< is not selected because it's in a different namespace than both std::priority_queue and std::tuple. It's not in the candidate set, so it is never even considered as an overload candidate.

The search for a suitable overload happens in the namespace where the operator is called from, which is the namespace priority_queue lives in, i.e. std. Argument-dependent lookup causes an additional search in the namespaces of the arguments, but because tuple is also in the std namespace, that doesn't help either. There is no reason for the compiler to ever consider the global namespace at all.

Instead, the standard library's definition of std::operator< for tuples is used. You can see that if you put your own implementation in a namespace std block:

// !!!!!!!!!!!!!!!!!!!!!!!!
// BAD SOLUTION, DO NOT USE
// !!!!!!!!!!!!!!!!!!!!!!!!

namespace std {
bool operator<(tuple<int, int, int> lhs, tuple<int, int, int> rhs)
{
    return get<1>(lhs) > get<1>(rhs); // Changed to > so we can see the difference.
}
}

Now the output is 3. Your operator is now considered and takes precedence over the default one, because non-template functions come before template functions.

But it is forbidden by the standard to put your own code into namespace std (with some small exceptions), so what to do? The solution is to pass a comparator functor explicitly:

#include <iostream>
#include <tuple>
#include <queue>
using namespace std;

struct element_1_greater {
    bool operator()(tuple<int, int, int> lhs, tuple<int, int, int> rhs)
    {
        return get<1>(lhs) > get<1>(rhs);
    }
};

int main()
{
    priority_queue<tuple<int, int, int>, vector<tuple<int, int, int>>, element_1_greater> q;
    q.push(make_tuple(2,5,3));
    q.push(make_tuple(2,3,3));
    cout << get<1>(q.top()) << '\n';
    return 0;
}

Notice that we need to pass the second argument to priority_queue as well; you can shorten this with a typedef or using declaration of course.

Thomas
  • 174,939
  • 50
  • 355
  • 478
  • Yeah, I showed the custom comparator template argument in the comments. I put it there, because it's not an actual answer to the question. And "best solution" is debatable. In my experience it's often "better" to replace the tuple by a custom class, if only to have proper names for the members. – JHBonarius Aug 16 '22 at 07:35
2

std::tuple is in the namespace std, the compiler looks for an overloaded operator< (3) in the namespace std until C++20 and operator<=> (7) also in the namespace std since C++20. The one of the global namespace is not considered while searching due to the argument dependent name lookup rules.

The fix:

#include <iostream>
#include <tuple>
#include <queue>
using namespace std;

bool operator<(tuple<int, int, int> lhs, tuple<int, int, int> rhs)
{
    return get<1>(lhs) > get<1>(rhs);
}

int main()
{
    using T = tuple<int, int, int>;
    priority_queue<T, vector<T>, bool(*)(T, T)> q(::operator<);
    q.push(make_tuple(2,5,3));
    q.push(make_tuple(2,3,3));
    cout << get<1>(q.top());
    return 0;
}
// Outputs: 3
273K
  • 29,503
  • 10
  • 41
  • 64
  • This answer would be better if it explained _why_ it's a closer match, with reference to the C++ overloading rules. In particular, non-template functions typically take precedence over template functions like the `std` implementation of `operator<`, so why isn't that happening here? – Thomas Aug 16 '22 at 07:11
  • Not a closer match, just OP's overload is even not in overload set with ADL. – Jarod42 Aug 16 '22 at 07:14
  • @Jarod42 Why not? Why wouldn't ADL apply here? – Thomas Aug 16 '22 at 07:15
  • 1
    @Thomas: ADL applies, `priority_queue` ans `tuple` bring namespace std, `int` brings no namespaces (and not the global namespace); So only `std::operator<` are found. – Jarod42 Aug 16 '22 at 07:18
  • @Jarod42 does adl take precedence? Because since the custom operator is defined in the global namespace I would have expected it to be found/considered as well. – Wutz Aug 16 '22 at 07:23