Skip to main content
gamedev.net
🔒 Locked

overloading +

Started by devronious Apr 8, 2009 at 11:16 AM 30 replies 2.9k views
Original Post
devronious
devronious
I'm having trouble with overloading operators. The code works but the return value is 0. The base type is a high precision quad double. If I debug and follow the code it does everything correct and even calculates the correct value. But something happens right when the + operator gets ready to return the value. It goes from an object with a correct value to an object that has zero like a brand new object made with the default constructor. Any ideas on what's happening? I have the following code: 
[source lang = "c++"]
public value struct QuadReal
{

	/*Variables*/
private:
	qd_real * value;

public:
	/*Constructors*/
	QuadReal(qd_real* n)
	{
		value = n;
	}
	QuadReal(double n) 
	{
		value = new qd_real(n);
	}     
	QuadReal(int n) 
	{
		value = new qd_real(n);
	}     

	/*Methods*/

public:		   

	virtual QuadReal & operator+ (const QuadReal &qr) override 
	{
		QuadReal* res = new QuadReal(0);
		res->value = &qd_real::ieee_add(*this->value, *qr.value);
		return *res;
	}


};
Thanks in Advance, Devin
SiCrane
SiCrane
Given the fact that the code you posted shouldn't even compile, it's hard to determine why the runtime behavior is weird. Though, if I had to guess, it's probably related to storing the address of a temporary.
jpetrie
jpetrie
This isn't C++, it's C++/CLI. Are you really intended to be writing C++/CLI?

There's a lot wrong with your code, but the way to correct it depends in some part on which language you think you're using.
loufoque
loufoque
What's that weird C++ look-alike language?
Is that C++/CLI?

From a C++ point of view, there are dozens of things wrong with your code.
Quite impressive actually with so few lines.

1) The first constructor seems highly dangerous. Make it explicit or better yet, copy the data.

2) Lacks a destructor that does delete value;

3) The default copy constructor and assignment operator will just break everything. Either re-implement them or disable them.

4) The operator+ declaration is not const-correct. It doesn't (and shouldn't anyway) modify "this".

5) Returning a reference to a heap-allocated object is just nonsense and is obviously a a leak. You're supposed to return by value.

6) When I look at your code, I see no reason why QuadReal::value is a qd_real* and not a qd_real. (except introducing many bugs and making your code less efficient, but I doubt that was intentional)

7) A virtual operator+ is weird at best. If you wanted polymorphic behaviour, it would have to depend on the types of the two operands, not just the left one.
devronious
devronious
OMG you guys figured out I'm new to c++, I was hoping to hide that :( JK

I've been studying this and I'm having trouble when searching the net to see much difference between c++ and c++/cli. Maybe its bad examples or something. What does a cli class look like? Anyone can point me to an example or something please?

Anyways, I'll address the numbered questions first:

1. qd_real is a struct and I thought simply assigning it would copy the struct instead of passing the address.

2. Again qd_real is destructed thru the stack isn't it? i tried puting a destructor and I got a compile error.

3.

4. That's what I thought but this example I found searching did just what I put in. So I've been steared wrong I guess. I thought maybe it modified it in the compiler to not be modified.

5. You mean using * instead of & after the return type? All the examples show it that way. Is that what you mean?

6. You mean not using a pointer? The underlying class there is native for sure, can I do that?

7. I thought maybe since it was a struct it had a virtual + in it already that was messing things up or something.

JPetrie,

I wanted to write a wrapper to use in c#. So I think I need CLI right?

SiCrane,

What do you mean a temporary?

Thanks guys for your help. I'm new to c++ as if you couldn't tell. And need some guidance.

-Devin
devronious
devronious
One question please... I thought when you construct a new class in c++/cli it must reside in a pointer, ie:

QuadReal* qr = new QuadReal(0);

Or is there a way to do this:

QuadReal qr = new QuadReal(0);
jpetrie
jpetrie
Quote:

OMG you guys figured out I'm new to c++, I was hoping to hide that :( JK

JPetrie,
I wanted to write a wrapper to use in c#. So I think I need CLI right?

If you want to expose something to C#, a C++/CLI class library is the way to go unless you can accomplish what you need via Platform Invoke. Unfortunately, C++/CLI has all the complexity of C++ and then some -- for somebody new to C++ alone, working in C++/CLI is an extremely bad idea because not only do you need to deal with C++ itself, you need to deal with additional layers of strangeness that appear because you're bridging the gap between the native and managed environment. It isn't a domain for the neophyte.

Quote:

I've been studying this and I'm having trouble when searching the net to see much difference between c++ and c++/cli. Maybe its bad examples or something. What does a cli class look like? Anyone can point me to an example or something please?

C++ is a native language. C++/CLI is a language that has native and managed parts, allowing you to create interoperability layers. This is a C++ class:
class ClassName {  public:    // variables and methods here};

A native class in C++/CLI looks the same. A managed reference-type class looks like:
public ref class ClassName {  public:    // variables and methods here};

The "public value struct" you used indicates a managed value type.

I assume you are trying to expose qd_real to C# code as QuadReal. If that's the case, pretty much everything about your code is wrong. First:

Quote:

1. qd_real is a struct and I thought simply assigning it would copy the struct instead of passing the address.

qd_real is a struct, but you're using qd_real* all over, which is a pointer-to-a-struct.

Quote:

2. Again qd_real is destructed thru the stack isn't it? i tried puting a destructor and I got a compile error.

Automatic variables are destroyed when execution leaves their scope. Whether or not a variable is automatic is a function of where that variable is declared, not its type. So "qd_real is destructed thru the stack" is false.

Managed code and types do not have detructors. They have finalizers. Value types cannot have user-defined finalizers, which is why the compiler complained, most likely).

Quote:

4. That's what I thought but this example I found searching did just what I put in. So I've been steared wrong I guess. I thought maybe it modified it in the compiler to not be modified.

The example may not have been valid for what you were trying to do. In any case managed types don't support the notion of C++ const-correctness anyway.

Quote:

5. You mean using * instead of & after the return type? All the examples show it that way. Is that what you mean?

He means returning 'QuadReal' and not QuadReal& or QuadReal*. The latter two are invalid for your purposes anyway, as managed code won't consume them the way you expect since they're on the native heap.

Quote:

7. I thought maybe since it was a struct it had a virtual + in it already that was messing things up or something.

Neither managed nor native types -- class or struct (which are equivalent except for default access rights) inherit default operator implementations. You don't need to implement operator+ as a virtual method, it won't "work" anyway.

Quote:

SiCrane,
What do you mean a temporary?

A temporary object. He is referring to this:
res->value = &qd_real::ieee_add(*this->value, *qr.value);

Here your call ieee_add, which presumably returns a qd_real, which is a temporary object. That object will cease to exist at some point in the near future when you return from the function, but you will have stored its address. The address will thus be referring to something that no longer exists.

Quote:

Thanks guys for your help. I'm new to c++ as if you couldn't tell. And need some guidance.

Wrapping this kind of object is tricky. Since you cannot store native structures by-value inside a managed type, you must heap-allocate the qd_real member. But you cannot set up a user-defined finalizer to ensure that you free the member, because this is a value type. You probably don't want to implement IDisposable for this object, as that will be painful to use (and won't really solve anything in the long run). You also probably don't want to make this a reference type if at all possible.

The best thing for you to do is show me the definition of qd_real -- what members does it contain? And explain to me why you feel you need to wrap this object. Then I'll have some suggestions.
SiCrane
SiCrane
The return value of ieee_add() is a qd_real. When you use & on that you're taking the address of a variable that lives on the stack. This is bad mojo because as soon as the full expression that contains that object is finished the return value will be destroyed and you're holding a pointer into nothingness.

If you want QuadReal to be a value type, then you probably shouldn't hold a qd_real by pointer inside it. However, since managed value types can't hold native objects or arrays you'll probably want to store the individual components of qd_real as members in the QuadReal struct. Ex:
public value struct QuadReal {private:  double v1;  double v2;  double v3;  double v4;public:  QuadReal(qd_real n) {    v1 = n.x[0];     v2 = n.x[1];     v3 = n.x[2];     v4 = n.x[3];   }  QuadReal(double n) {    qd_real temp(n);    v1 = temp.x[0];     v2 = temp.x[1];     v3 = temp.x[2];     v4 = temp.x[3];   }  QuadReal(int n) {    qd_real temp(n);    v1 = temp.x[0];     v2 = temp.x[1];     v3 = temp.x[2];     v4 = temp.x[3];   }public:  static QuadReal operator+(const QuadReal lhs, const QuadReal rhs) {    qd_real t1(lhs.v1, lhs.v2, lhs.v3, lhs.v4);    qd_real t2(rhs.v1, rhs.v2, rhs.v3, rhs.v4);    return QuadReal(qd_real::ieee_add(t1, t2));  }};
jpetrie
jpetrie
Quote:

One question please... I thought when you construct a new class in c++/cli it must reside in a pointer, ie:

QuadReal* qr = new QuadReal(0);

Or is there a way to do this:

QuadReal qr = new QuadReal(0);

"new" creates an object on the native heap. Except for CLI simple value types (which your type is), using new to construct an instance actually makes your program ill-formed.

In this case it is legal, but will put the instance on the native heap. To create the instanced on the managed heap you use a handle and gcnew:
T^ t = gcnew T();

But it is not true that you always need a pointer (or handle). For value types, like this one, you don't.
devronious
devronious
Yea I was adding the pointers accidentally. I fixed it to do the values instead:

[source lang = "cpp"]		public value struct QuadReal		{			/*Variables*/		private:			qd_real* value;		public:			/*Constructors*/			QuadReal(qd_real* n)			{				*value = *n;			}			QuadReal(double n) 			{				value = new qd_real(n);			}     			QuadReal(int n) 			{				value = new qd_real(n);			}     			/*Methods*/					   		public:		   			QuadReal & operator+(const QuadReal &qr)			{				QuadReal* res = new QuadReal(0);				*res->value = qd_real::ieee_add(*this->value, *qr.value);				return *res;			}		};


jpetrie,

I've tried creating it like:

QuadReal qr = new QuadReal(0);

and I get an error so I'm not sure how to create it without a pointer to it. I'll use a handle instead.

Quote:
I assume you are trying to expose qd_real to C# code as QuadReal.


Yes that's correct. OK so no finalizer since its a struct native. The code is huge so I'll point you to the government page where it resides, which is again why I wanted to use a pointer or struct to simplify wrapping:

QD for QuadReal


SiCrane,

I would rather just use a pointer to the struct or a struct if that's possible, can I use something other than a pointer to qd_real?
NotAYakk
NotAYakk
So there are some idioms for writing standard operators. Following them makes things much easier.

First, write the following operators:
class Foo {public:  Foo& operator+=( Foo const& other) {    // modify yourself.    return *this;  }  // similarly, implement these:  // Foo& operator-=  // Foo& operator*=  // Foo& operator/=  // swaps the internal data of *this and other.  void swap( Foo& other );  // Please please be careful about who owns what  // pointers in the following classes:  // Constructs a copy of other:  Foo( Foo const& other);  // destroys the data owned by Foo.  virtual ~Foo();  // Implement this ordering operation:  bool operator<( Foo const& other ) const { ... }  // optionally implement this ordering operation, or use the generic one  // implemented below:  bool operator==( Foo const& other ) const { ... }  // Now, you get easy implementations of a lot of members:  Foo operator+( Foo const& other ) const {    Foo tmp = *this; // copy construct    return tmp += other;  }  // Foo operator- ... etc  // The copy-swap idiom for operator=:  Foo& operator=( Foo const& other ) {    Foo tmp = other;    this->swap(tmp);    return *this;  }  // easy ordering operations:  bool operator==( Foo const& other ) const { return !(*this < other) && !(other < *this); }  bool operator> ( Foo const& other ) const { return other < *this; }  bool operator<=( Foo const& other ) const { return !(*this > other); }  bool operator>=( Foo const& other ) const { return !(*this < other); }  bool operator!=( Foo const& other ) const { return !(*this == other); }};


While the above idioms are not ideal for every case, they do isolate the easier to write methods and implement the other methods in terms of the other operators.

The above assumes you are implementing _value semantics_ -- your constructor owns the data, it doesn't just refer to it. This is in comparison to pointer/reference semantics.

Reference semantics runs into problems with a number of the operators. You end up having to add in a value-semantics like clone to implement a number of them...

If your data is expensive to duplicate, copy on write can be used to allow temporary duplicates to be created cheaply. If you go multi-threaded, that gets more complex.
SiCrane
SiCrane
Quote:
Original post by devronious
I would rather just use a pointer to the struct or a struct if that's possible, can I use something other than a pointer to qd_real?

If you want QuadReal to be a managed value type, then your options are really limited. You can't store a native struct by value in a managed value type and if you try storing a pointer you're going to run into problems with managing resources since managed value types cannot have user defined assignment operators or destructors.
SiCrane
SiCrane
Quote:
Original post by NotAYakk
So there are some idioms for writing standard operators. Following them makes things much easier.

Unfortunately, those are for normal C++, not C++/CLI. Ex: managed value types can't have a user defined operator=().
NotAYakk
NotAYakk
Really? Damn. I guess I never wrote a C++/CLI operator=.

Isn't C++/CLI the language that uses ^ for managed pointers and gcnew? Or am I thinking of another one of the C++/.net hybrids microsoft put out? (It has been a while, and I was just generating wrapper code instead of writing it manually)
devronious
devronious
Jpetrie,

[qoute]
"new" creates an object on the native heap. Except for CLI simple value types (which your type is), using new to construct an instance actually makes your program ill-formed.

In this case it is legal, but will put the instance on the native heap. To create the instanced on the managed heap you use a handle and gcnew:

T^ t = gcnew T();
[/qoute]

So how do I create it in the stack? Even if a do a = operator it will use new or gcnew wont it?
SiCrane
SiCrane 
QuadReal qr; // creates a default constructed QuadReal on the stack
SiCrane
SiCrane
Quote:
Original post by NotAYakk
Isn't C++/CLI the language that uses ^ for managed pointers and gcnew?

Yes.
Quote:
Or am I thinking of another one of the C++/.net hybrids microsoft put out?

That would be managed C++, which is the version that had the __gc keyword used practically every third lexeme.
loufoque
loufoque
Your answer to my numbered remarks show it all. You really need to learn the basics, get a book maybe.

1) Unrelated to the remark, and also irrelevant. The issue I raised here is ownership transfer.
2) It's a pointer, an address in memory. That address is the address of an object you allocated with new. That memory isn't going to be automatically freed, no. Though why you did allow that memory with new is quite a mystery.
4) Learn about the const qualifier as applied to member functions. QuadReal operator+(const QuadReal& q) const, for example.
5) * is the dereference operator. I mean the fact that you're returning a reference to an object that was allocated on the heap and thus will never be freed.
6) You can always use direct storage and not a pointer, certainly. C++ is not a stupid language where every variable has to be a pointer.
7) I don't understand the comment.

Anyway, since this is really trivial, here is what you should have written:

struct QuadReal{    template<typename T>    QuadReal(const T& n) : value(n)    {    }    friend QuadReal operator+(const QuadReal& q1, const QuadReal& q2)    {        return qd_real::ieee_add(q1.value, q2.value);    }private:    qd_real value;};


This assumes the copy constructor and assignment operator of qd_real are correct and don't lead to dangerous aliasing.
Since qd_real is a C++ type, as it has a constructor in your example, that should probably be the case.

Note how this is merely a qd_real wrapper that overloads operator+. You might as well overload operator+ for qd_real directly.
no such user
no such user
Quote:
Original post by loufoque
Anyway, since this is really trivial, here is what you should have written:

Except that idea is to create a QuadReal value type usable from C# and your code won't work for that.
devronious
devronious
OK, this is what I ended up with. I learned alot so far. Can you tell me if anything is dangerously wrong with this, by that I mean it will corrupt or something later on:

[source lang = "cpp"]		public value struct QuadReal		{			/*Variables*/		private:			qd_real* value;		public:			/*Constructors*/			QuadReal(qd_real &n)			{				this->value = &n			}			QuadReal(double n) 			{				this->value = new qd_real(n);			}     			QuadReal(int n) 			{				this->value = new qd_real(n);			}     			/*Methods*/					   		public:		   			//copy operator			static QuadReal operator-(const QuadReal lhs)			{				QuadReal qr = QuadReal(0);				*qr.value = *lhs.value->negate();				return qr;			}						static QuadReal operator+(const QuadReal lhs, const QuadReal rhs)			{				QuadReal res = QuadReal(0);				*res.value = qd_real::ieee_add(*lhs.value, *rhs.value);				return res;			}			static QuadReal operator+=(const QuadReal lhs, const QuadReal rhs)			{				QuadReal res = QuadReal(0);				*res.value = qd_real::ieee_add(*lhs.value, *rhs.value);				return res;			}			static QuadReal operator-(const QuadReal lhs, const QuadReal rhs) 			{				QuadReal res = QuadReal(0);				*res.value = qd_real::ieee_add(*lhs.value, *rhs.value->negate());				return res;			}			static QuadReal operator-=(const QuadReal lhs, const QuadReal rhs)			{				QuadReal res = QuadReal(0);				*res.value = qd_real::ieee_add(*lhs.value, *rhs.value->negate());				return res;			}			static QuadReal operator*(const QuadReal lhs, const QuadReal rhs) 			{				QuadReal res = QuadReal(0);				*res.value = qd_real::accurate_mul(*lhs.value, *rhs.value);				return res;			}			static QuadReal operator*=(const QuadReal lhs, const QuadReal rhs)			{				QuadReal res = QuadReal(0);				*res.value = qd_real::accurate_mul(*lhs.value, *rhs.value);				return res;			}			static QuadReal operator/(const QuadReal lhs, const QuadReal rhs) 			{				QuadReal res = QuadReal(0);				*res.value = qd_real::accurate_div(*lhs.value, *rhs.value);				return res;			}			static QuadReal operator/=(const QuadReal lhs, const QuadReal rhs)			{				QuadReal res = QuadReal(0);				*res.value = qd_real::accurate_div(*lhs.value, *rhs.value);				return res;			}			static QuadReal Sqrt(QuadReal n)			{				qd_real qr = std::sqrt(n.value);				return QuadReal(qr);			}			virtual String^ ToString()  override			{				int digits = 3;				std::string s =					value->to_string(					digits, 					0, 					static_cast<std::ios_base::fmtflags>(0),					false, false, 					32); 				String^ ns = gcnew String(s.c_str());				return ns;			}		};


Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.