Function::operator+ is missing

[Chaffra Affouda]

Now that Function::operator+ is removed in commit 49b7e1ad160 how can I add two functions into a FunctionAXPY. Should FunctionAXPY implement FunctionAXPY::operator=(const FunctionAXPY&) so that operation can be performed on the same FunctionAXPY object?

[Jan Blechta]

The operator has been removed because of the conversion of reference to nodelete pointer.

There are dozens of ways how this can be done

FunctionAXPY result(first, second, FunctionAXPY::Direction::ADD_ADD);

FunctionAXPY result(first, 1.0);
result += second;

FunctionAXPY result({{1.0, first}, {1.0, second}});

FunctionAXPY result({});
result += first;
result += second;

[Jan Blechta]

Oh, ok I see your point. There is only operator+ not operator += etc.

But still the first and the third way suggested above should work (with correct types).

[Chaffra Affouda]

Yes that's correct. I could not find FunctionAXPY::operator= or += etc... I don't see an elegant way of making a sum in a for loop without them. Actually this is breaking a lot of things in my code.

[Chaffra Affouda]

Maybe the Function::operator 's should be left in place and instead Function should inherit from std::enable_shared_from_this<Function>. That way you can call Function::shared_from_this() to generate a new shared_ptr to this. I don't know if that will take care of the memory corruption with NoDeleter.

[Jan Blechta]

I don't see an elegant way of making a sum in a for loop without them.

Come on, there's

    /// Constructor
    FunctionAXPY(
      std::vector<std::pair<double, std::shared_ptr<const Function>>> pairs)    ;

First construct pairs in loop.

std::enable_shared_from_this seems to be like a good suggestion. That's similar to what I was thinking about at #643 but I was not aware of its presence in std.

[Chaffra Affouda]

Ha like I said: not elegant :-). But thanks for reminding me of that constructor. Also since Function is a Hierarchical it has a _self member which is already a shared_ptr. You might just use it directly instead of creating a new one every time a Function::operator is called. I tried it and it works.

And I just realized your are aware of the _self member in issue #643.

[Jan Blechta]

Good point, let's

• revert 49b7e1ad160,
• use Hierarchical::_self instead of reference_to_no_delete_pointer(*this) temporarily,
• fix #643 (hopefully using std::enable_shared_from_this),
• use shared_from_this() instead of Hierarchical::_self.

[Jan Blechta]

std::enable_shared_from_this::shared_from_this() is undefined behaviour in C++11 when object is not managed by shared ptr. This is fixed in C++17 and boost which throw bad_weak_ptr in this case.

[Jan Blechta]

There are also some corner cases of bad interaction with no-deleters, see http://stackoverflow.com/questions/29259500. We could expect a lot of problems with enable_shared_from_this.

[Chaffra Affouda]

So the workaround for now would be:

** use Hierarchical::_self instead of reference_to_no_delete_pointer(*this) inside the operators.

[Jan Blechta]

That's not acceptable as long as we are not able to make Hierarchical::_self safe w.r.t. memory management. (enable_shared_from_this looks promising but it might open another can of worms.)

Please, propose any other solution which is safe, preferably in the form of a pull request. Extension of FunctionAXPY interface might do the job for you.

[Chaffra Affouda]

Hi Jan, I am a little swamped with work but I'll get to this ASAP.