microsoft / Range-V3-VS2015

A fork of the popular range-v3 C++ library with support for the Visual Studio 2015 Update 3 VC++ compiler.
Other
115 stars 22 forks source link

concept check for find fails with UDTs #9

Closed aligature closed 7 years ago

aligature commented 7 years ago

The following code produces a template instantiation error in the concept check before the operator() functions in struct find_fn. Removing the concept check allows the code to compile. The same thing happens in the second example while searching with a compatible/comparable type.

I've attached the error output: range_error.txt

#include <range/v3/all.hpp>
#include <vector>

struct Foo
{
   std::string name;
   friend bool operator==(Foo const& lhs, Foo const& rhs) { return lhs.name == rhs.name; }
};

int main()
{
   auto f = std::vector<Foo>{ {"foo"}, {"bar"}, {"car"}, {"jar"} };
   auto foo = Foo{ "foo" };
   auto it = ranges::find(f, foo);
   it = ranges::find(f.begin(), f.end(), "foo", &Foo::name);

   auto ss = std::vector<std::string>{ "foo", "bar", "car", "jar" };
   auto it2 = ranges::find(ss, "nora");

   return 0;
}
CaseyCarter commented 7 years ago

There are a couple of interesting things going on here:

  1. auto it = ranges::find(f, foo); To prevent the proliferation of tiny, nearly meaningless concepts that plagued the C++0x concepts design, concepts in the Ranges TS are usually clusters of semantically related requirements. EqualityComparable, the concept that's stepping on your toes here, requires both == and != to be defined (and in fact requires them to be consistent with each other as well: (t != u) == !(t == u) must hold). Since Foo only defines ==, it doesn't satisfy EqualityComparable and hence this call to find is not valid. The fix is to define operator !=.
  2. it = ranges::find(f.begin(), f.end(), "foo", &Foo::name); This falls into a bit of a design hole in the Ranges TS: "foo" has array type, and the concepts are ill equipped for dealing with relations between arrays and non-arrays. (I've opened ericniebler/stl2#183 to track this issue.) If you pass char*s or std::strings instead of arrays to the algorithms, you can avoid the issue for now, e.g.:
it = ranges::find(f.begin(), f.end(), +"foo", &Foo::name);
/*...*/
auto it2 = ranges::find(ss, +"nora");

Works because the string literals are forcibly decayed to pointers with unary +.

ErnestPenfold commented 7 years ago

Had the same problem and adding a operator!= worked. But if the type to compare with is not the same as in the range the error is back. (I added operator== and operator!= for this type) Because std::find works fine in this situation I assume the range find should, too, shouldn't it?

Simple code for demonstration (no meaning intended):

#include <vector>
#include <range/v3/algorithm.hpp>

int main()
{
  class X
  {
  public:
    X() {}
    X(int _i) : i(_i){}
    X(X&&) = default;
    X(const X&) = default;

    bool operator==(const int v) const   {
      return v == i;
    }
    bool operator!=(const int v) const   {
      return v != i;
    }
    bool operator==(const X& x) const   {
      return x.i == i;
    }
    bool operator!=(const X& x) const   {
      return x.i != i;
    }

    int i;
  };

  std::vector<X>  v;
  X x(9);

  // works
  std::find(v.begin(), v.end(), 9);
  // works
  ranges::v3::find(v, x);
  // error
  ranges::v3::find(v, 9);
  // works
  ranges::v3::find(v, 9,  /* lambda init here - will be killed be editor*/
  {
    return x.i;
  });

  return 0;
}
ericniebler commented 7 years ago

Given the definitions you show, you're missing the ability to say 42 == X(42). Ditto for !=.

ErnestPenfold commented 7 years ago

Thanks, added both missing comparision operators and now it compiles and works. In fact running this sample only operator==(int) is actually called - which meets my expectations. I did not dig too deep into the concepts applied to find. Could you please briefly explain why there are so strong requirements (stronger than std::find) to use ranges::find which lead to many unused functions. Thanks in advance.

CaseyCarter commented 7 years ago

Could you please briefly explain why there are so strong requirements (stronger than std::find) to use ranges::find which lead to many unused functions.

To prevent the proliferation of tiny, nearly meaningless concepts that plagued the C++0x concepts design, concepts in the Ranges TS are clusters of semantically related requirements. Cross-type EqualityComparable, the concept in question here, requires all of:

to be defined, and further requires those operations to be consistent. Which of those expressions a given algorithm requiring EqualityComparable<T, U>() happens to use is an implementation detail.

aligature commented 7 years ago

I think the fundamental issues have been reasonably explained. Thanks for all the info.