Problem with own global operator+

[Emanuel Ziegler]

Hello,

I want to do some mathematics with functions. In my case the function
classes are very complex, but this simple example has the same problems.

To allow calculations that begin with a double, I have to define a global
operator+ (or -*/) and special function classes.

--- source begins here ---

#include <iostream>
using namespace std;

class function {
// private:
// function *f;
public:
function (function *F = 0) {
cout << "constructor" << endl;
// f = F;
}
virtual ~function () {
cout << "destructor" << endl;
// delete f;
}
virtual double operator() (double x) {
// if ( f == 0 )
cout << "ERROR!" << endl;
// else
// return (*f)(x);
};
};
class identity : public function{
public:
virtual double operator() (double x) { return x; }
};
class sum : public function {
private:
double a;
function *b;
public:
sum (double A, function &B) : function() { a = A; b = &B; }
virtual double operator() (double x) { return a+(*b)(x); }
};

function & operator+ (double x, function &B) {
return *new sum(x,B);
}

int main () {
identity id;
cout << (1+id)(1) << endl;
cout << (1+(1+id))(1) << endl; // comment out for case 3
return 0;
}

--- source ends here ---

There are three cases:
(1) Run the program as it is. You should get:

constructor
constructor
2
constructor
constructor
3
destructor

As you can see, this creates memory leaks!

(2) Comment out all lines in the declaration of function and
change the definition of operator + to

function operator+ ...
return new ...

i.e. remove the first & in the first line and the * in the second line
of the definition.

This program will _not_ compile! On g++ I get

src/prog/testfunc.cpp: In function `int main()':
src/prog/testfunc.cpp:43: error: no match for 'operator+' in '1 +
operator+(double, function&)((&id))'
src/prog/testfunc.cpp:36: error: candidates are: function operator
+(double,function&)

I have no problems with member operators doing calculations like
(function+1)+2
it's only the global operators!

(3) Same as in case 2, but also comment out the marked line in case 3 and
you should get

constructor
constructor
constructor
2
destructor
destructor
destructor

My questions are now:
1. Does anybody know a better solution than case 3 for avoiding memory
leaks?
2. If not, how can I get case 2 to work (without changing int main(),
of course)?
Thanks in advance
Emanuel

[Emanuel Ziegler]

Emanuel Ziegler wrote:

(2) Comment out all lines in the declaration of function and
change the definition of operator + to

Of course, you must remove the comments in the declaration of function.

Sorry
Emanuel

[Rolf Magnus]

Emanuel Ziegler wrote:

Hello,

I want to do some mathematics with functions. In my case the function
classes are very complex, but this simple example has the same
problems.

To allow calculations that begin with a double, I have to define a
global operator+ (or -*/) and special function classes.

--- source begins here ---

#include <iostream>
using namespace std;

class function {
// private:
// function *f;
public:
function (function *F = 0) {
cout << "constructor" << endl;
// f = F;
}
virtual ~function () {
cout << "destructor" << endl;
// delete f;
}
virtual double operator() (double x) {
// if ( f == 0 )
cout << "ERROR!" << endl;
// else
// return (*f)(x);
};
};
class identity : public function{
public:
virtual double operator() (double x) { return x; }
};
class sum : public function {
private:
double a;
function *b;
public:
sum (double A, function &B) : function() { a = A; b =
&B; }
virtual double operator() (double x) { return
a+(*b)(x); }
};

function & operator+ (double x, function &B) {
return *new sum(x,B);
Very bad idea. Who deletes the created object? You should never return
references to dynamically allocated memory. Why don't you simply return
by valu anyway?
}

int main () {
identity id;
cout << (1+id)(1) << endl;
cout << (1+(1+id))(1) << endl; // comment out for case 3
return 0;
}

--- source ends here ---

There are three cases:
(1) Run the program as it is. You should get:

constructor
constructor
2
constructor
constructor
3
destructor

As you can see, this creates memory leaks!
Yes. The reason is that you allocate objects with new, but never delete
them.
(2) Comment out all lines in the declaration of function and
change the definition of operator + to

function operator+ ...
return new ...

i.e. remove the first & in the first line and the * in the
second line of the definition.
Again, you should _not_ use new at all. Is it possible that you are
coming from a Java backround? Proper usage of new in C++ is completely
different from Java.
This program will _not_ compile! On g++ I get

src/prog/testfunc.cpp: In function `int main()':
src/prog/testfunc.cpp:43: error: no match for 'operator+' in '1
+
operator+(double, function&)((&id))'
src/prog/testfunc.cpp:36: error: candidates are: function
operator
+(double,function&)

I have no problems with member operators doing calculations like
(function+1)+2
it's only the global operators!
The result of (function+1) is a temporary, and you can't bind non-const
references to temporaries in C++. When you don't modify the parameter,
make it a const reference. I think that's your problem here.
(3) Same as in case 2, but also comment out the marked line in case
3 and
you should get

constructor
constructor
constructor
2
destructor
destructor
destructor

My questions are now:
1. Does anybody know a better solution than case 3 for avoiding
memory leaks?
2. If not, how can I get case 2 to work (without changing int
main(),
of course)?
Thanks in advance
Emanuel

--
"Have you got an extra goto 10 line?"
(from Futurama)

[Emanuel Ziegler]

Rolf Magnus wrote:

function & operator+ (double x, function &B) {
return *new sum(x,B);

Very bad idea. Who deletes the created object? You should never return
references to dynamically allocated memory. Why don't you simply return
by valu anyway?

I get an error message, when returning "sum(x,B)", because it's (obviously)
temporary.

A correct return by value, using "function operator+ ..." and "return
sum(x,B);" is also impossible, since "sum" enhances "function" and an
assignment "function" = "sum" would need a typecasting constructor. But
1. I cannot create a typecasting constructor, since "sum" is defined
later and therefore not known to the compiler when defining "function".
2. The compiler wouldn't reserve enough space to store the private
pointers of "sum".

As you can see, this creates memory leaks!

Yes. The reason is that you allocate objects with new, but never delete
them.

I know. That's why I implemented case 2. A garbage collector is a very good
invention, but unfortunately not known to c++. That's the reason for the
complicated definition of "function" with the "delete" line in the
destructor.

function operator+ ...
return new ...

Again, you should _not_ use new at all. Is it possible that you are
coming from a Java backround?

Parts of Java and Object Pascal. From the latter I'm used to deleting
objects created with "new". See the reasons for "new" above.

I have no problems with member operators doing calculations like
(function+1)+2
it's only the global operators!

The result of (function+1) is a temporary, and you can't bind non-const
references to temporaries in C++. When you don't modify the parameter,
make it a const reference. I think that's your problem here.

Making the parameter constant does not solve the problem, because I have to
make the second parameter of the "sum"-constructor constant. Then I have to
make the pointer "b" inside "sum" constant. Finally, I cannot call
operator() of "*b".

Emanuel

[Alberto Barbati]

Emanuel Ziegler wrote:

A correct return by value, using "function operator+ ..." and "return
sum(x,B);" is also impossible, since "sum" enhances "function" and an
assignment "function" = "sum" would need a typecasting constructor. But
1. I cannot create a typecasting constructor, since "sum" is defined
later and therefore not known to the compiler when defining "function".
2. The compiler wouldn't reserve enough space to store the private
pointers of "sum".

You have clearly identified the source of the problem. By inheriting sum
from function you can no longer use function by-value.

What I suggest is to make function a proxy class and to make a separate
inheritance tree for functions. Like this:

class function_base {
private:
virtual ~function ()
{ }

virtual double operator() (double x) = 0; // virtual pure
};

class function {
private:
boost::shared_ptr<function_base> f;

public:
explicit function (function_base *F) // explicit is safer
: f(F)
{}

// no destructor needed, as boost::shared_ptr calls delete

double operator() (double x) const // notice this is const!!
{
return (*f)(x);
}
};

boost::shared_ptr is a reference counter smart pointer class
<http://www.boost.org/libs/smart_ptr/smart_ptr.htm>. you can use any
other smart pointer class of your choice but *not* std::auto_ptr,
otherwise function would not copiable.

Now you derive sum from function_base (*not* function):

class sum : public function_base {
private:
double a;
function b; // by value

public:
sum (double A, const function& B) : a(A), b(B) {}
virtual double operator() (double x) { return a+b(x); }
};

and write your operator+ as:

function operator+ (double x, const function& B) {
return function(new sum(x, B));
}

notice that return is now by value. The parameter B could have been
passed by value insted of by const& as function has correct
value-semantic and not very expensive to copy.

I have no problems with member operators doing calculations like
(function+1)+2
it's only the global operators!

The result of (function+1) is a temporary, and you can't bind non-const
references to temporaries in C++. When you don't modify the parameter,
make it a const reference. I think that's your problem here.

Making the parameter constant does not solve the problem, because I have to
make the second parameter of the "sum"-constructor constant. Then I have to
make the pointer "b" inside "sum" constant. Finally, I cannot call
operator() of "*b".

My approach works around the problem. The temporary (of type function)
is constant but you *can* invoke function::operator() that is now
declared const. BTW, are you sure it won't be better to declare
function_base::operator() const as well?

My €0.01

Alberto Barbati

[Emanuel Ziegler]

Alberto Barbati wrote:

What I suggest is to make function a proxy class and to make a separate
inheritance tree for functions. Like this:
This doen't work, either, as I will explain below. I made some corrections
that are not listed here, because they are not relevant for the problem.
See source below for details.
class function {
...
double operator() (double x) const // notice this is const!!
This is the critical part. It is impossible to overwrite operator() for some
reason. Why does g++ behave like this?
...
function b; // by value
Here the compiler fails, since the abstract operator() has not been
overwritten. Thus, instances cannot be created.
My approach works around the problem. The temporary (of type function)
is constant but you *can* invoke function::operator() that is now
declared const. BTW, are you sure it won't be better to declare
function_base::operator() const as well?

Making function_base::operator() constant does not solve the problem,
either. Although, it is impossible to compile the code without doing this.
With this correction and making operator() non-abstract, I can compile, but
not execute the program.

Here's my corrected source (execution may *crash* on some machines):

--- source begins here ---

#include <iostream>
#include <boost/shared_ptr.hpp>
using namespace std;

class function_base {
public:
virtual ~function_base () {}
// My version:
virtual double operator() (double x) const {};
// Version proposed by Alberto Barbati:
// virtual double operator() (double x) = 0;
};
class function : public function_base {
private:
boost::shared_ptr<const function_base> f;
public:
function (const function_base *F) : f(F) {}
virtual double operator() (double x) {
return (*f)(x);
}
};
class identity : public function_base {
public:
virtual double operator() (double x) { return x; }
};
class sum : public function_base {
private:
double a;
function b;
public:
sum (double A, const function_base &B)
: function_base(), a(A), b(&B) {}
virtual double operator() (double x) { return a+b(x); }
};

function operator+ (double x, const function_base &B) {
return function(new sum(x,B));
}

int main () {
identity id;
cout << (1+id)(1) << endl;
return 0;
}

--- source ends here ---

The output is

nan
memory protection fault

The first line indicates, that operator() has not been overwritten (no
return statement in original version). The second one doesn't look better.

Emanuel

[Alberto Barbati]

Emanuel Ziegler wrote:

class function {
...
double operator() (double x) const // notice this is const!!

This is the critical part. It is impossible to overwrite operator() for some
reason. Why does g++ behave like this?

Could you please give more details (for example giving the exact error
message) why g++ would not want you to override operator+()?

...
function b; // by value

Here the compiler fails, since the abstract operator() has not been
overwritten. Thus, instances cannot be created.

operator() is not virtual in my suggestion, it need not and should not
be virtual.
Here's my corrected source (execution may *crash* on some machines):
Your "corrected" source??? You disrupted the source! Class function
should *NOT* derive from function_base. That's why you are having
problems! Why did you do that?
class sum : public function_base {
[...]
sum (double A, const function_base &B)
: function_base(), a(A), b(&B) {}
[...]
};

function operator+ (double x, const function_base &B) {
return function(new sum(x,B));
}

in both cases the parameter should be of type const function&, not const
function_base&.

Alberto

[Emanuel Ziegler]

Alberto Barbati wrote:

Your "corrected" source??? You disrupted the source! Class function
should *NOT* derive from function_base. That's why you are having
problems! Why did you do that?

Uuh, stupid me! Sorry, but I overread this important point in your source.
Of course, the source didn't compile when deriving function from
function_base. This was the reason for my unnecessary "corrections".

Now, I got it working. Anyway, I still don't understand c++'s behaviour with
the other source.

Thanks a lot!
Emanuel