views:

218

answers:

5

I have the following tricky problem: I have implemented a (rather complicated) class which represents mathematical functions in a multiwavelet basis. Since operations like +, - and * are quite natural in this context, I have implemented overloaded operators for this class:

FunctionTree<D> operator+(FunctionTree<D> &inpTree);
FunctionTree<D> operator-(FunctionTree<D> &inpTree);
FunctionTree<D> operator*(FunctionTree<D> &inpTree);

There operators work finer in simple, non-chained operations, and even in some cases when chaining the operators. Statements like

FunctionTree<3> y = a * b + c;
FunctionTree<3> z = a * b + b;

compile and seemingly work fine. The first line is actually ok, but the second line causes valgrind to tell you grim stories about memory being freed inside already freed regions and uninitialized variables being accessed. Furthermore, a statement like

FunctionTree<D> y = a + b * c;

will not even compile, because I have not defined (an ambiguous operator taking an actual object, not a reference, as argument). Of course the solution is clear: All arguments, and methods ought to be made const, and perhaps even return a const object or reference. Unfortunately this is not possible, since NONE of the objects involved are constant during the operations! This may sound strange, but this is an unavoidable consequence of the mathematics involved. I can fake it, using const_cast, but the code is still wrong!

Is there a way to solve this problem? The only solution I have currently is to make the return objects const, thus effectively disabling the operator chaining.

Regards, .jonas.

+2  A: 

There is no simple solution to this problem. Your binary operators are (correctly) producing nameless temporary objects - such objects cannot be bound to non-const reference parameters.

One possible way round this is to forgo the chaining of operators - for a class X:

X a;
X b;
X c = a * b;
X d;
X e  = c + d;

Another (rather horrible) solution is to make the the data members of the class mutable - that way they would be logically const but physically alterable. You would then make your parameters be const references. Like I said, this is horrible and may cause other problems.

anon
This is precisely what I'm doing now, by making the return values const, so that chaining is not possible.
Jonas Juselius
A: 

Pass the arguments by value rather than by reference?

EDIT: You may want to use += -= *= instead.

Ari
if that was possible, it could be passed as const ref as well. From what I understood, OP wants to modify the input parameter as well.
Naveen
I don't think so. He wants to be able to modify them during computation (e.g., to hold temporary values). I don't think he wants the inputs to be actually modified. I might be wrong.
Ari
This is not really an option, since these objects can be really, really big (order of +1 GB in size for molecular densities), which make them costly.
Jonas Juselius
That didn't seem to be a concern in the return value.
Ari
+1  A: 

"...NONE of the objects involved are constant during the operations! This may sound strange..." No it doesn't sound strange, it sounds wrong. If your operators modify the objects in a way that is observable from the outside, then that's an abuse of operator overloading. If the objects are modified, because results are cached, make the cache mutable, so functions modifying the cache can still be declared const.

Henrik
The changes are not observable from the outside; The objects being added or multiplied need to be temporarily brought into a state which allows them to be added or multiplied (they need to have identical tree structures). When the operation is completed, the auxiliary data is purged.
Jonas Juselius
In such case **mutable** should apply. Also make sure that you haven't turned logically local variables into class members.
UncleBens
Ok, so I briefly looked at using mutable. No go ;) My classes are very complex, and there is a lot of data and auxiliary quantities to keep track of. The mutable data is therefore not directly contained in the tree structure, but rather in a special tracking container class, which provides the right objects when needed. The whole thing just got very tricky to implement, so I dropped it...
Jonas Juselius
So... just make the tracking container member mutable? Maybe I'm misunderstanding what you're saying.
Alan
+5  A: 

If your objects are 1GB (I guess that's memory they allocate on the heap, not their actual sizeof size), you probably shouldn't be supporting these operators on them. The problem is that your examples of operator chaining more or less presume immutable objects as its basic model of "what's going on", and create lots of temporaries to serve as intermediate results. You can't count on the compiler to be able to re-use the space efficiently. But you also can't copy 1GB objects willy-nilly.

Instead, you should only support the various assigning operators. Then your client instead of writing:

y = a * b + c;

which is liable to create enormous temporaries, should instead write:

// y = a * b + c
y = a;
y *= b;
y += c;

That way the user can keep a lid on things. It's easily seen that no temporaries are created, and you won't accidentally write a simple line of code that requires 18GB to run. If you want to do:

y = a * b + c * d;

then your code must explicitly note that a temporary result is required here (assuming you can't trash any of a, b, c, d):

// y = a * b + c * d
y = a;
y *= b;
{
   FunctionTree x = c;
   x *= d;
   y += x;
}

but if the caller happens to know that for example c isn't needed after this, you can explicitly do:

// y = a * b + c * d  [c is trashed]
c *= d;
y = a;
y *= b;
y += c;

In theory the compiler might be up to working all this stuff out for itself based on big expression with the chained operators, and data-flow analysis to show that c is unused. Good compilers with lots of optimisation switched on are good at this for integer arithmetic, so the machinery is there. In practice I wouldn't count on it. If the compiler cannot prove that the constructor and destructor of FunctionTree have no observable side-effects, then its ability to skip them is limited to the specific cases of legal "copy elision" in the standard.

Or you could look at the C interface to GMP to see how this is done without operator overloading at all. All the functions there take an "out parameter", which the result is written to. So for example if x is a huge multiple precision integer, that you want to multiply by 10, you choose whether to write:

mpz_mul_ui(x, x, 10); // modifies x in place, uses less memory

or:

mpz_t y;
mpz_init(y);
mpz_mul_ui(y, x, 10); // leaves x alone, result occupies as much memory again.
Steve Jessop
I think there's also a thing called Expression Templates which might help against these problems. :)
UncleBens
That's new to me, but if Boost Linear Algebra uses it, then I guess that it's equally applicable to operations on functions. A Banach space is just a big vector space, after all.
Steve Jessop
+1  A: 

You can use proxies instead of real values, and proxies can be constant, as they are not going to be changed. Below is a small example of how it might look like. Be aware that all the temporaries are still going to be created in that example but if you want to be smart, you can just save the operations, not the actual results of operations, and calculate only when someone wants to finally get result or a part of it. It might even speed up your code enormously as it helped APL

Also, you might want to make most of the members private.

#include <memory>
#include <iostream>

struct FunctionTreeProxy;
struct FunctionTree;

struct FunctionTreeProxy {
    mutable std::auto_ptr<FunctionTree> ft;

    explicit FunctionTreeProxy(FunctionTree * _ft): ft(_ft) {}
    FunctionTreeProxy(FunctionTreeProxy const & rhs): ft(rhs.ft) {}

    FunctionTreeProxy operator+(FunctionTree & rhs);
    FunctionTreeProxy operator*(FunctionTree & rhs);
    FunctionTreeProxy operator+(FunctionTreeProxy const & rhs);
    FunctionTreeProxy operator*(FunctionTreeProxy const & rhs);
};

struct FunctionTree {
    int i;
    FunctionTree(int _i): i(_i) {}
    FunctionTree(FunctionTreeProxy const & rhs): i(rhs.ft->i) {}

    FunctionTree * add(FunctionTree & rhs) {
        return new FunctionTree(i + rhs.i);
    }

    FunctionTree * mult(FunctionTree & rhs) {
        return new FunctionTree(i * rhs.i);
    }

    FunctionTreeProxy operator+(FunctionTree & rhs) {
        return FunctionTreeProxy(add(rhs));
    }

    FunctionTreeProxy operator*(FunctionTree & rhs) {
        return FunctionTreeProxy(mult(rhs));
    }

    FunctionTreeProxy operator+(FunctionTreeProxy const & rhs) {
        return FunctionTreeProxy(add(*rhs.ft));
    }

    FunctionTreeProxy operator*(FunctionTreeProxy const & rhs) {
        return FunctionTreeProxy(mult(*rhs.ft));
    }
};

FunctionTreeProxy FunctionTreeProxy::operator+(FunctionTree & rhs) {
    return FunctionTreeProxy(ft.get()->add(rhs));
}

FunctionTreeProxy FunctionTreeProxy::operator*(FunctionTree & rhs) {
    return FunctionTreeProxy(ft.get()->mult(rhs));
}

FunctionTreeProxy FunctionTreeProxy::operator+(FunctionTreeProxy const & rhs) {
    return FunctionTreeProxy(ft.get()->add(*rhs.ft));
}

FunctionTreeProxy FunctionTreeProxy::operator*(FunctionTreeProxy const & rhs) {
    return FunctionTreeProxy(ft.get()->mult(*rhs.ft));
}

int main(int argc, char* argv[])
{
    FunctionTree a(1), b(2), c(3);

    FunctionTree z = a + b * c;

    std::cout << z.i << std::endl;

    return 0;
}
vava
Certainly worth considering!
Jonas Juselius
I tried your example (which wouldn't compile due to some strange, strange issues with the auto_ptr), and I think this is exactly what I need, if and when I decide to allow chaining again. Thank you very much!
Jonas Juselius
You might need to be a bit careful with this - the pointer indirection basically disables const correctness for the class. Whether an FTProxy is `const` or not becomes irrelevant, because the FT is non-const anyway. If the reason for FTProxy was PImpl, you'd "fix" that with private "getPtr" function, with const and non-const versions returning `const FT*` and `FT*`. Since you don't want const-correctness FTProxy's fine, but be aware what you're doing, which is subverting `const`. It's better than making every single data member `mutable`, but worse than making only selected members `mutable`.
Steve Jessop
@Steve, it is not entirely true. You still won't be able to use `const FT` variables in those expressions just because they are not allowed as parameters for any of the operators. And you won't be able to convert them to `FTProxy` without explicit `const_cast`. So const correctness is still there. Proxy just prolongs life of temporary objects and makes them not so much temporary.
vava
@Jonas, it compiles in VC++ :)
vava
@Jonas, but you have to `#include <memory>` to get `std::auto_ptr`
vava
Oh, sorry, I may have misunderstood what's going on here.
Steve Jessop
Btw, I think the issue with VC++ compiling vs. Jonas having a problem with auto_ptr is that when you return a FunctionTreeProxy by value, the standard says the copy constructor must be accessible (even though here, copy elision can kick in). The auto_ptr means that FTProxy's implicit copy ctor takes a non-const reference (12.8/5), which can't be bound to a temporary. VC++ presumably is only checking that a copy ctor exists (one does). GCC and Comeau both check that the *right* copy ctor exists, one that could be called to do the copy. It doesn't.
Steve Jessop
@Steve, that's true. I was thinking about using boost::shared_ptr but then decided std::auto_ptr is good enough. Not always apparently :) I'll check if compiling with gcc possible this weekend.
vava
Well, I just made it a normal pointer, and added a destructor. It seemed to work fine, and created no leaks :) Thank you guys for a very fruitful discussion!
Jonas Juselius
@Jonas, be careful with default copy constructor. If you haven't created your own or disabled default one by making it private, you might have a lot of strange issues with double deletion of objects, as default copy constructor just copy your pointer value. `std::auto_ptr` was much safer as it transfer the ownership during copy and that's exactly why you had problems with it :) So you might see really strange crashes or memory corruptions.
vava
Yep, what vava says. If you didn't get double frees with the change you describe, it must be that it just so happens, all the copies of FTProxy objects in this particular code can be avoided with copy elision. That won't be true in general, so you'd need to write a copy constructor for FTProxy.
Steve Jessop
Fixed compilation in gcc 4.4.1. FTProxy now owns a pointer just as std::auto_ptr does, so you can't pass it around as parameter or use basically anywhere where copying is involved except for temporary objects as they are short lived and not reusable even by compiler.
vava
But, unlike with auto_ptr, compiler won't stop you from doing stupid things as const correctness for FTProxy is broken. It is still there for FT though.
vava