1

EDIT: This is not a duplicate question. People are confusing my question as "why does my code not work" but its not that, its "why does it work on VS 2013 and not 2019" and that has been answered. This is not a duplicate question. I stated multiple times asking why it works in 2013 and not 2019.

So I was creating a DLL, and decided to upgrade from VS2013 to VS2019 so I would get the most recent visual studio packages and the most up to date IDE of course. Out of nowhere a error became apparent after upgrading: initial value of reference to non-const must be an lvalue Since this error I have finally managed to fix it, but I would like to understand why this error happens, especially after upgrading to a different version of visual studio.

Simply put, the code that was working in visual studio 2013 was a vector structure with member functions and operator overloading as follows:

HEADER

struct Vec2 {
    float x, y;
    /* Insert other obligatory code */
    Vec2 add(const Vec2);
    Vec2 &operator+(const Vec2);
}

IMPLEMENTATION

Vec2 Vec2::add(const Vec2 o) {
    return {
        x + o.x,
        y + o.y
    };
}
Vec2 &Vec2::operator+(const Vec2 o) {
    return add(o); // <-- 'initial value of reference to non-const must be an lvalue'
}

Like I said before, this code works and compiles perfectly. After I upgraded it was throwing the before mentioned error when it reached return add(o); inside my overloaded plus operator function.

I just wanted to know why this started happening AFTER I upgraded... did something change in the MSVC compiler?

I did fix it though, this is my solution, but I am afraid that it might be inefficient...

NEW HEADER

struct Vec2 {
    /* yada yada */
    Vec2 &add(const Vec2);
    Vec2 &operator+(const Vec2);
}

NEW IMPLEMENTATION

Vec2 &Vec2::add(const Vec2 o) {
    Vec2 v(x + o.x, y + o.y); // Just normal constructor
    return v;
}
Vec2 &Vec2::operator+(const Vec2 o) {
    return add(o); 
}

Any knowledge shared will be greatly appreciated, and if there is a better, faster, and/or easier solution to the one I implemented, please tell me! :)

Joseph
  • 127
  • 2
  • 10
  • Why do you return by reference? – Yksisarvinen Aug 24 '19 at 13:42
  • 3
    `operator+()` should not return a reference type – Michael Nastenko Aug 24 '19 at 13:43
  • 2
    It's an issue with older versions of VS, they allow non-standard behaviour by default. In this case binding a non-const reference to an rvalue. Note that you have UB if you try to use the reference return by `Vec2::operator+` as the object referred to has already be destroyed. – Richard Critten Aug 24 '19 at 13:49
  • In addition, you should have overloaded `+=` first, and then write `operator +` in terms of `+=`. It is `+=` that would return the reference, not `+`. – PaulMcKenzie Aug 24 '19 at 13:52
  • 3
    Your code was always broken. You simply upgraded to a smarter compiler that told you that your code is broken. – Sam Varshavchik Aug 24 '19 at 13:55
  • I know the problem, and I greatly appreciate the tips, but I mostly stressed in my question why it worked fine in VS2013 and not VS2019, and currently @Richard Critten has answered my question pretty well. – Joseph Aug 24 '19 at 13:55
  • 2
    If it worked fine in VS2013 it's only because that compiler itself was also broken, and it did not comply with the C++ standard. – Sam Varshavchik Aug 24 '19 at 13:56
  • 1
    @Joseph -- If you turned the warning level up to 4 in VS 2013, it should have reported to you that the syntax was non-standard. – PaulMcKenzie Aug 24 '19 at 13:57
  • Okay that clears things up perfectly then. I wish I could point out richard's paul's, and sam's comments as answers but since there not like posts I cant, and I don't want to answer my own question and take the credit :S – Joseph Aug 24 '19 at 13:58
  • smh some one reads first couple sentences and marks it as a duplicate. – Joseph Aug 24 '19 at 14:05
  • @Joseph: The problem is this: even if we answer your question and explain where your code was wrong and why MSVC properly diagnoses it now, you *still need* the duplicate question because it will tell you how to *correctly* do operator overloading. Because your "fixed" version is still returning a reference to a destroyed variable. So yes, it doesn't really answer your question, but it is something you *need to read and follow*. – Nicol Bolas Aug 24 '19 at 14:09
  • @Nicol Bolas, but my question was why it didnt work in visual studio 2019 and how it seemingly worked on 2013, not how to do operator overloading. The other people in this comment stream such as richard, paul, and sam have perfectly answered the question, I just don't know how to mark them as the answer. – Joseph Aug 24 '19 at 14:11
  • In addition, literally the title of my question has nothing to do with operator overloading, "Updating VS2013 to VS2019 causes Lvalue error, would like to understand why" – Joseph Aug 24 '19 at 14:12
  • @Joseph: "*but my question was why it didnt work in visual studio 2019*" Which is why *I reopened* your question. The point you're not getting is that you still need the duplicate because you're still *doing it wrong*. You are still writing bad code and need to be corrected. It may not be the answer to your question, but it's information that you need to know, because your code is *still broken*. – Nicol Bolas Aug 24 '19 at 14:22
  • @Joseph: "*I just don't know how to mark them as the answer.*" Comments are not answers, so you cannot "mark them as the answer." – Nicol Bolas Aug 24 '19 at 14:24
  • @NicolBolas Its not whether I wrote bad code, I could have given that bad code has an example, it doesn't matter. I was asking why certain code worked in a certain version of visual studio. I was not specifically asking how to fix my code. Thats literally it. – Joseph Aug 24 '19 at 14:24

1 Answers1

2

These member functions are incorrectly declared and defined

Vec2 add(const Vec2);
Vec2 &operator+(const Vec2);

For example in the first function the qualifier const in the parameter declaration is redundant. The functions should be declared with the qualifier const because they does not change the original vector.

In the second function you are returning a reference to a temporary object.

The functions can be declared and defined the following way

struct Vec2 {
    float x, y;
    /* Insert other obligatory code */
    Vec2 add(const Vec2 &) const;
    Vec2 operator+(const Vec2 & ) const;
};

Vec2 Vec2::add(const Vec2 &o) const {
    return {
        x + o.x,
        y + o.y
    };
}

Vec2 Vec2::operator+(const Vec2 &o) const {
    return add(o); 
}

Here is a demonstrative program

#include <iostream>

struct Vec2 {
    float x, y;
    /* Insert other obligatory code */
    Vec2 add(const Vec2 &) const;
    Vec2 operator+(const Vec2 & ) const;
};

Vec2 Vec2::add(const Vec2 &o) const {
    return {
        x + o.x,
        y + o.y
    };
}

Vec2 Vec2::operator+(const Vec2 &o) const {
    return add(o); 
}

int main() 
{
    Vec2 v1 = { 10, 10 };
    Vec2 v2 = { 20, 20 };

    Vec2 v3 = v1 + v2;

    std::cout << v3.x << ' ' << v3.y << '\n';

    v3 = v3.add( v3 );

    std::cout << v3.x << ' ' << v3.y << '\n';

    return 0;
}

Its output is

30 30
60 60
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335