operator+() code for objects
in [C++]
|
Prev: help on round robin tournament
Next: sizeof: Why parentheses needed when used with data types (when not required with variables)
From: Paul Bibbings on 8 Nov 2009 03:17 silentway wrote: > I am trying to create a simple vector class with overloaded operators > for assignment, +=, -=, +, *, etc > My code for the overloaded operators worked OK, until I tried operator+ > () > The problem is that I can not return a local variable. The problem here is not actually that you cannot return a local variable, but rather that you do not have a copy constructor that can perform the required copy initialization of F from the value returned by your overload operator+() (see below). > This is fair > enough, but I am sure that this used to work ok 10 years ago, when I > was using Zortech compiler. I am back to C++ after an outrageously > long break without doing any codng. I am now using g++ compiler. I was not seriously coding in C++ 10 years ago and have never used the Zortech compiler, but as 10 years leaves us still within the time of the C++98 standard, I very much doubt that any compiler that aimed to be standards-compliant (if Zortech ever did) would have compiled the code as is. > class vec > { > public: // <snip> > vec(vec &rv) : mx(rv.mx) { printf("in copy constr\n"); } // ; // <snip> > const vec operator+(const vec &rv); // <snip> > }; Here, your copy constructor takes its argument by non-const reference, whereas the return value from your overloaded op+ is a const rvalue which will fail to bind on both counts. <snip> > int main () { > vec A(0.5); A.print("A:"); > vec B(0.1); B.print("B:"); // <snip> > // vec F = A + B; // problem!! > //error: no matching function for call to 'vec::vec(vec)' > return 0; > } (From your code as given, I would have expected the error message to have been error: no matching function for call to 'vec::vec(const vec)'. Removing the const from the return value of your op+ would be needed to produce the error you give here.) > The class and test code works OK except for the line > vec F = A + B; In this line we have a call to your op+ followed by copy initialization of F. There must be a valid copy constructor that can handle the result of op+ in order to perform the initialization (even where the compiler might choose to elide it). As mentioned, your op+ returns a const rvalue, whereas your copy constructor as given is looking for a non-const lvalue. Removing the const from the return value of op+ is not sufficient, as we still have an rvalue which will not bind to the non-const reference of your copy constructor. What you need is: vec::vec(const vec& rv); > The error message is > error: no matching function for call to 'vec::vec<const vec>' > > I have seen lots of examples that use the above code (or similar), but > they don't seem to work for me > I am starting to suspect that gcc / g++ does not support something > here, but that sounds too crazy gcc is doing fine! ;-) Regards Paul Bibbings -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Torsten Robitzki on 8 Nov 2009 03:17 silentway schrieb: > // vec F = A + B; // problem!! > //error: no matching function for call to 'vec::vec(vec)' <snip> > Can someone point me in the right direction? I hope so. The problem is you copy contructor taking a refernce to not const vec. The term A + B creates a temporary and a temporary can't be bound to a reference to not const vec. vec(const vec &rv) : mx(rv.mx) { printf("in copy constr\n"); }; ^^^^^ will solve the problem. Btw. the usual implementation of operator+ would be a free function and be implemented in terms of operator+= best regards Torsten -- kostenlose Wirtschaftssimulation: http://www.financial-rumors.de [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Neil Butterworth on 8 Nov 2009 03:16 silentway wrote: My comments are inline with te code: > #include <iostream> Your use of printf() (why not use iostreams?) eans that you should include the header it s declared in: #include <cstdio> > > class vec > { > public: > vec(float x = 0.0) : mx(x) { printf("in constructor\n"); }; > vec(vec &rv) : mx(rv.mx) { printf("in copy constr\n"); }; This is your main problem. A copy constructor should take a const reference as it's parameter - you want: vec( const vec &rv) : mx(rv.mx) { printf("in copy constr\n"); }; > virtual ~vec() {}; > void setx(float x) {mx = x;}; > vec & operator=(const vec &rv); > vec & operator+=(const vec &rv); > const vec operator+(const vec &A) /*const*/; > void print(char *msg); > private: > float mx; > }; > > vec & vec::operator=(const vec &rv) { > printf("in vec & vec::operator=(const vec &rv)\n"); > if (this != &rv) > mx = rv.mx; > return *this; > } > > vec & vec::operator+=(const vec &rv) { > printf("in vec & vec::operator=(const vec &rv)\n"); > mx += rv.mx; > return *this; > } > > // this compiles ok, but produces error if overloaded + is used in > code > const vec vec::operator+(const vec &A) /*const*/ { > printf("in vec vec::operator+(const vec &A)\n"); > vec result(*this); > result += A; > return result; > } There is no need for the return value of this method to be const. Also, operator + is normally implemented as a free function, not a member, to allow conversion of the LHS operand. > void vec::print(char *msg) { > printf("%s %f\n", msg, mx); > } Once again, why printf instead of iostreams? > int main () { > vec A(0.5); A.print("A:"); > vec B(0.1); B.print("B:"); > vec C(B); C.print("C:"); > vec D = A; D.print("D:"); > vec E; E.print("E:"); > E = A; E.print("E:"); > B += A; B.print("B:"); > // vec F = A + B; // problem!! > //error: no matching function for call to 'vec::vec(vec)' Fixed by making the copy ctor take a const reference, so it can bind to the const value returned by operator+ > return 0; > } > > The class and test code works OK except for the line > vec F = A + B; > > I am compiling from the command line as follows > g++ -g vec.cpp -o vec -lm > > The error message is > error: no matching function for call to 'vec::vec<const vec>' > > I have seen lots of examples that use the above code (or similar), but > they don't seem to work for me > I am starting to suspect that gcc / g++ does not support something > here, but that sounds too crazy Always, always suspect your own code rather than a compiler bug. Neil Butterworth -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Matthew Collett on 8 Nov 2009 03:22 In article <bea3b28d-235b-4b0d-8421-9fef3ecb8272(a)a31g2000yqn.googlegroups.com>, silentway <andybolsover(a)yahoo.co.uk> wrote: > I am trying to create a simple vector class with overloaded operators > for assignment, +=, -=, +, *, etc > My code for the overloaded operators worked OK, until I tried operator+ > () .... > Here is my code > > #include <iostream> > > class vec > { > public: > vec(float x = 0.0) : mx(x) { printf("in constructor\n"); }; > vec(vec &rv) : mx(rv.mx) { printf("in copy constr\n"); }; And here is your problem. Your copy constructor should take its argument by constant reference: vec(const vec &rv) : mx(rv.mx) { printf("in copy constr\n"); }; Currently you cannot copy const objects, and so the compiler complains about a missing copy constructor. Best wishes, Matthew -- http://homepages.ihug.co.nz/~m_collett [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Ulrich Eckhardt on 8 Nov 2009 05:29
silentway wrote: > I am trying to create a simple vector class with overloaded operators > for assignment, +=, -=, +, *, etc > My code for the overloaded operators worked OK, until I tried > operator+(). The problem is that I can not return a local variable. Huh? > This is fair enough, but I am sure that this used to work ok > 10 years ago, when I was using Zortech compiler. I am back to > C++ after an outrageously long break without doing any codng. Get a good book. Many things changed. > #include <iostream> > > class vec > { > public: > vec(float x = 0.0) : mx(x) { printf("in constructor\n"); }; > vec(vec &rv) : mx(rv.mx) { printf("in copy constr\n"); }; > virtual ~vec() {}; > void setx(float x) {mx = x;}; > vec & operator=(const vec &rv); > vec & operator+=(const vec &rv); > const vec operator+(const vec &A) /*const*/; > void print(char *msg); > private: > float mx; > }; Several things here: 1. Make one-argument constructor 'explicit' to avoid accidental construction. 2. Do not create a virtual destructor for objects you don't plan to use polymorphically. 3. You #include <iostream> but use printf() for which you didn't include any header. 4. What is print()? Why does it take a pointer to a non-const char? What does it to the memory pointed to by it? 5. Do not blindly terminate lines with a semicolon. You don't need them after the definition of a function. I consider it a quirk, but it is actually allowed in the standard when defining a function as part of the class definition, but not outside. 6. Your copy-constructor takes a reference to a non-const object. This is most surely wrong and this oversight is also causing you problems. > const vec vec::operator+(const vec &A) /*const*/ { > printf("in vec vec::operator+(const vec &A)\n"); > vec result(*this); > result += A; > return result; > } The copy-constructor that doesn't take a reference to const is what breaks this. When fixed, you could also make the operator const. However, there is another thing you should know, and that is that operators don't have to be defined as memberfunctions: vec operator+(vec const& v1, vec const& v2) { vec res(v1); res += v2; return res; } This is the typical implementation for operator+ based on operator+=, which needs to remain inside the class. Note that you can also overload a free operator for the combination vec/float or float/vec, which wouldn't work for a bound one. > g++ -g vec.cpp -o vec -lm Add -Wall Uli -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |