3

I am using a vector with my class type A as its element type. I defined the relational operator < for my class but when I compare two of these vectors it crashes.

class A {
    public:
        explicit A(int){}
        bool operator<(const A&)const {
            return true; // for simplicity
        }
};


int main() {

    std::vector<A> v{ A(1), A(2), A(3), A(4), A(5) };
    std::vector<A> v2{ A(0), A(2), A(4), A(6) };

    std::cout << (v < v2) << std::endl; // program crashes here!

}

I am trying to understand this because I've read that the STL containers use the element type's relational operator?!

  • the program runs fine on GCC but on MSVC++ 14 it crashes so I get the assertion dialog complaining about the line std::cout << (v < v2) << std::endl; // program crashes here!

  • The most interesting thing as @user4581301 pointed out is if I define the relational operator correctly it solves the problem:

    class A {
        public:
            explicit A(int) {}
            bool operator<(const A& a)const {
                return x < a.x; // for simplicity
            }
            int x;
    };
    

Now it works fine and the program doesn't crash! Is this some restriction in MSVC++? (I mean defining the operator correctly?)

Boann
  • 48,794
  • 16
  • 117
  • 146
Itachi Uchiwa
  • 3,044
  • 12
  • 26
  • 1
    @MaxVollmer: It crashes on MSvc++ 15! – Itachi Uchiwa Aug 12 '19 at 22:12
  • 1
    Reproduced. Error message: Expression: invalid comparator – user4581301 Aug 12 '19 at 22:17
  • 2
    Okay, then include that info and the error message in your question :) – Max Vollmer Aug 12 '19 at 22:19
  • 3
    Could be upset because A < B is true as is B < A. Heck, A – user4581301 Aug 12 '19 at 22:19
  • @user4581301: Thank you! it solves the problem!!! please add it as an answer. – Itachi Uchiwa Aug 12 '19 at 22:21
  • As described in the [documentation](https://en.cppreference.com/w/cpp/container/vector/operator_cmp), the `A` should meet the requirements of [LessThanComparable](https://en.cppreference.com/w/cpp/named_req/LessThanComparable). – Algirdas Preidžius Aug 12 '19 at 22:23
  • Why voting down then as long as this is a problem in MSVC not me? also any beginner can face such problem. – Itachi Uchiwa Aug 12 '19 at 22:25
  • 3
    MSVC has some code (that I think only runs in debug builds) to validate the comparison operators. If it detects a contradiction in the ordering you'll get the "invalid comparator" assertion. – 1201ProgramAlarm Aug 12 '19 at 22:26
  • 5
    Can't answer. 1) question's locked. 2) Not much to answer here. The error message said exactly what was wrong. You over-simplified the comaprator, broke a language rule, and Visual Studio caught the error for you. I call that a win for Visual Studio. – user4581301 Aug 12 '19 at 22:27
  • 1
    @ItachiUchiwa "Why voting down then as long as this is a problem in MSVC not me?" 1) I, personally, downvoted for the lack of research. Since the requirements for the type, to be used in such a context, is outlined in the documentation (as linked above). The first place to look, when one is having problems with using certain API. 2) 99% of times, it's the fault of the person, and not the compiler. If you will be so quick to jump to conclusions, that it can't be you, you won't get far in software developing.. – Algirdas Preidžius Aug 12 '19 at 22:27
  • @user4581301: But why GCC allows such error as the OP asked? – Alex24 Aug 12 '19 at 22:29
  • 2
    @Alex24 "_But why GCC allows such error?_" 1) What does "allow such error" mean? 2) Undefined behavior, being undefined? – Algirdas Preidžius Aug 12 '19 at 22:30
  • 2
    Nothing in the Standard requires that this be checked. You'll find the Standard littered with "No diagnostic required" notes. Likely this is hard to conclusively detect at compile time. Note how it compiles without error or warning. Visual Studio's debugger throws you a bone here and checks at runtime, otherwise you probably wouldn't notice the problem until a sort looped forever or triggered a stack overflow. – user4581301 Aug 12 '19 at 22:34
  • @user4581301: Ok. Thank you! I understood now. – Itachi Uchiwa Aug 12 '19 at 22:37
  • 2
    Side note: General wisdom suggests `operator <` be implemented as a free function rather than a method. See [What are the basic rules and idioms for operator overloading?](https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading) for explanation and more wisdom. – user4581301 Aug 12 '19 at 22:37
  • 1
    If you use `return false;` for simplicity it would work, `return true;` is a bad choice here – Slava Aug 12 '19 at 22:58

2 Answers2

8

I defined the relational operator < for my class "A" then when I create vector of A objects and compare these two vectors it crashes?!

You chosen a bad case here, if you use return false; then you should be fine as it simply mean that all instances of A is equal to each other. return true; makes it bad operator.

the program runs fine on GCC but on Msvc++ 14 it crashes

No it does not run fine. Problem with Undefined Behavior is that one of the unfortunate behavior that program appear to work fine until you change something even unrelated. Like changing compiler or even version of the same or compiler optimization level.

Now it works fine and the program doesn't crash! Is this some restriction in MSVC++?

It is not a restriction it just happened. Compilers in C++ are not obligated to ensure that programer did not create UB it is programer job not to create it, that is the price for efficiency. Even more compiler may assume that there is no UB, and there are known cases that some code was eliminated because of such case.

Slava
  • 43,454
  • 1
  • 47
  • 90
  • 2
    maybe you could add just a single line to explain why `return true;` makes the operator a bad one. – Federico Aug 13 '19 at 11:00
4

In

class A {
    public:
        explicit A(int){}
        bool operator<(const A&)const {
            return true; 
        }
};

The comparator is too simple and allows misbehavior like A < B and B < A to both be true, along with A < A. This is obviously flawed logic and Visual Studio's special debug libraries are testing for it and trapping it to make the programmer aware of the error.

A more complicated example like

class A {
    int x;
public:
    explicit A(int val):x(val) {}
    bool operator<(const A& cmp)const {
        return x < cmp.x; // for simplicity
    }
};

will demonstrate the point the Asker was trying to test.

Notes

I cannot find anywhere in the C++ Standard that requires operator < to be logically implemented (This doesn't mean there isn't one. The document is long and written for high-level multi-class Programmer/Lawyers). This certainly has no requirement for validation and a diagnostic at at compile time or we'd be seeing error messages or warnings from the compiler. I do not believe testing at the compiler level would be desirable as the code complexity to support this would be nightmarish.

C++'s Standard Sorting functions DO require sensible behaviour from comparison functions as do containers (specifically [tab:container.opt]), but no diagnostic is required and probably not tested for the reasons above. Visual Studio's runtime checking is a nice bone they're throwing programmers to help trap mistakes that could lead to A swapping with for B and then swapping back again forever resulting in an infinite loop or stack overflow in a sorting algorithm.

On the more practical and less legal level, a less-than operator that does not provide a less-than result violates the Law of Least Surprise, will result in confusing code, and will be used incorrectly far too often to be worth it. In general an overloaded operator should do what the operator is expected to do. Nothing more and nothing less.

Community
  • 1
  • 1
user4581301
  • 33,082
  • 7
  • 33
  • 54