Peter Hull
Peter Hull

Reputation: 7067

How can I specify an overloaded operator in a different namespace?

I'm having trouble with the C++ standard library. The following example does not compile: (note this is cut down to make a minimal example so does not make much sense as it is)

#include <algorithm>
#include <string>
#include <vector>

namespace otherns {

class Property {
public:
  const std::string &getName() const { return m_name; }

private:
  std::string m_name;
};
}

bool operator==(const otherns::Property &a, const otherns::Property &b) {
  return a.getName() == b.getName();
}

/* Merge, second takes priority */
std::vector<otherns::Property>
merge_props(const std::vector<otherns::Property> &xs,
            const std::vector<otherns::Property> &ys) {
  std::vector<otherns::Property> ans = ys;
  for (const auto &x : xs) {
    if (std::find(ans.begin(), ans.end(), x) == ans.end()) {
      ans.push_back(x);
    }
  }
  return ans;
}

The error is "binary '==': no operator found which takes a left-hand operand of type 'otherns::Property' (or there is no acceptable conversion)" which occurs somewhere in the implementation of std::find. This is with MSVC but I also tried with clang and gcc, with a similar result.

The following code does work:

std::vector<otherns::Property>
merge_props(const std::vector<otherns::Property> &xs,
            const std::vector<otherns::Property> &ys) {
  std::vector<otherns::Property> ans = ys;
  for (const auto &x : xs) {
    if (std::find_if(ans.begin(), ans.end(), [&x](const otherns::Property &y) {
          return x == y;
        }) == ans.end()) {
      ans.push_back(x);
    }
  }
  return ans;
}

I suppose this is something to do with ADL/Koenig lookup but I don't really understand why my operator== is not found. what's the best solution if I want to use the first, simpler form of the find function?

In reality the otherns comes from a header for a 3rd party library so I can't put my operator into that header.

Upvotes: 6

Views: 1305

Answers (3)

Rama
Rama

Reputation: 3305

Just declare operator== inside the namespace otherns (The lookup will find it at namespace scope)

namespace otherns {
bool operator==(const otherns::Property &a, const otherns::Property &b) {
  return a.getName() == b.getName();
}
}

Working code

You can do it in a separate header of the 3rd party library.

Upvotes: 1

bolov
bolov

Reputation: 75688

The rules are quite complicated, and I myself don't fully grasp them, but let's see if we can make head or tails of them (I think we can):

namespace nx {
struct X {};
}

namespace ns {    
auto foo(nx::X x1, nx::X x2) { return x1 == x2; }
// error: no match for 'operator==' (operand types are 'nx::X' and 'nx::X')
}

auto operator==(nx::X, nx::X) { return true; }

auto global_foo()
{
  return ns::foo(nx::X{}, nx::X{}); 
}

This isn't found for a simple reason: operator== is not declared before its use. Nothing to to with ADL yet. So far so good. We understand that. Let's fix it:

namespace nx {
struct X {};
}

auto operator==(nx::X, nx::X) { return true; }

namespace ns {
auto foo(nx::X x1, nx::X x2) { return x1 == x2; }
}

auto global_foo()
{
  return ns::foo(nx::X{}, nx::X{}); 
}

Does this work? Yes, it does, it compiles and calls our operator==. Is this the correct solution? No!. Because if we add this:

namespace nx {
struct X {};
}

auto operator==(nx::X, nx::X) { return true; } // (1)

namespace ns {

template <class T> auto operator==(T, int) { return false; } // (2)

auto foo(nx::X x1, nx::X x2) { return x1 == x2; }
// error: no match for 'operator==' (operand types are 'nx::X' and 'nx::X')
}

auto global_foo()
{
  return ns::foo(nx::X{}, nx::X{});
}

Then (2) in ns hides (1) in global namespace, even if (1) would be a better fit. This is called name hiding and - again - doesn't involve ADL in any way.

Even worse:

namespace nx {
struct X {};
}

auto operator==(nx::X, nx::X) { return true; } // (1)

namespace ns {

template <class T> auto operator==(T, T) { return false; } // (2)

auto foo(nx::X x1, nx::X x2) { return x1 == x2; } // calls (2)
}

auto global_foo()
{
  return ns::foo(nx::X{}, nx::X{}); 
}

Would compile and silently calls (2) instead of our operator (1).

For a real world context think of namespace ns as namespace std and any operator declared inside std. And you've got the situation in your post.

The correct solution is:

namespace nx {
struct X {};
auto operator==(nx::X, nx::X) { return true; } // (1)
}

namespace ns {

template <class T> auto operator==(T, T) { return false; } // (2)

auto foo(nx::X x1, nx::X x2) { return x1 == x2; } // calls (1)
}

auto global_foo()
{
  return ns::foo(nx::X{}, nx::X{}); 
}

What happens here is that ADL kicks in and brings (1) from nx and now (1) is considered alongside (2). But (1) is more specialized than (2) and so (1) is correctly selected.

If you don't have control of namespace nx and can't add the operator there, then what I can advise is to use callables instead of relying on operators. E.g instead of std::find use std::find_if with your own predicate (lambda) where you control exactly which method/operator to call. And when I say "exactly" I mean exactly: i.e. ::operator==(x1, x2) (or whatever namespace you declared them) instead of x1 == x2.


You can read more on this great article by Herb Sutter Namespaces & Interface Principle

Upvotes: 2

Toby Speight
Toby Speight

Reputation: 30705

You defined operator== in the global namespace (perhaps misled by the misindentation). It won't be found there by argument-dependent lookup.

The operator should be declared in the same namespace as (one of) its arguments:

namespace otherns {

    class Property {
    public:
        const std::string &getName() const { return m_name; }

    private:
        std::string m_name;
    };

    bool operator==(const otherns::Property &a, const otherns::Property &b) {
        return a.getName() == b.getName();
    }
}

That little change enables your example to compile cleanly.

Upvotes: 0

Related Questions