saving a lambda expression as parameter/variable inside a class to another class

Question

I made a class and a struct.

The class is named Learning and the struct is named Action.

My Action constructor takes one parameter: object's function, and the function is a std::function<int(int)>.

This is my Action struct:

typedef std::function<int(int)> func;

struct Action {
    // constructor
    Action(func);

    /// methods
    /// operators
    int operator()(int x);

    /// members
    func f;
};

Action(func f) {this->f = f; }

My Action struct is used by my Learning class by calling this function:

class Learning
{
public:
    void addAction(Action);
    Action getAction(int idx);
private:
    std::vector<Action> actions;
};

void Learning::addAction(Action act)
{
    actions.push_back(act);
}

int Learning::getAction(int idx)
{
   return actions[idx];
}

int main(){
Learning robot;
robot.addAction(Action([](int y) ->int{return y++; }));
std::cout << robot.getAction(0)(0) << std::endl;
return 0;
}

Where the Action is saved inside an actions vector in my Learning class:

The method addAction() adds the created Action object into my actions vector. another method 'getAction(idx)' is used to call one action from action vector.

I used a lambda expression as the parameter because it looks cleaner.

But when I call robot.getAction(0)(0), or actions[0](0) inside the class, I get an exception:

Unhandled exception at 0x00007FFA4DE44F69 in RL_Q.exe: Microsoft C++ exception: std::bad_function_call at memory location 0x000000C09A7BE4C0.

When I debug this, my function f is empty after I instantiate my Action object with given parameters.

How do I solve this?

Not the crashing issue but `return y++;` returns the original value of `y`. Just `return y+1;` no need for anything fancy. — Mike Vine, Dec 01 '21 at 20:21
I suspect that this isn't your real code, as `void Learning::addAction(Action& act)` will not accept the r-value (temporary) that you claim to be passing to it. A [mre] is likely necessary to answer this question. — Drew Dormann, Dec 01 '21 at 20:28
my ```void Learning::addAction(Action& act)``` is an lvalue. the rvalue is inside the constructor Action(); — Zain Ahmed, Dec 01 '21 at 20:31
@ZainAhmed `addAction()` takes a non-const lvalue reference for its parameter, so it will NOT accept a temporary object as input, as you have shown. The compile fails: "*cannot bind non-const lvalue reference of type ‘Action&’ to an rvalue of type ‘Action’*". To accept a temporary object, the parameter needs to take either a const lvalue reference (`const Action&`), or an rvalue reference (`Action&&`). — Remy Lebeau, Dec 01 '21 at 20:32
This is Visual Studio's fault, thanks to a "language extension" that allows binding temporaries to non-`const` references: https://stackoverflow.com/questions/16380966/non-const-reference-bound-to-temporary-visual-studio-bug — alter_igel, Dec 01 '21 at 20:36
Even so, the `push_back()` should be making a *copy* of the passed `Action`, even if the original is a temporary. — Remy Lebeau, Dec 01 '21 at 20:41
@ZainAhmed do you have the same problem if you change the `Action` constructor to take `func` by value instead of by const reference? `Action(name, func)`. Also, is the `Action` constructor even assigning the `f` member? You didn't show the constructor implementation — Remy Lebeau, Dec 01 '21 at 20:45
sorry this is my constructor: ``` RL::Action::Action(name name, func func) { setname(name); setfunction(func); } where setname is a function that set name and setfunction sets the f ``` — Zain Ahmed, Dec 01 '21 at 21:10
@ZainAhmed please add the relevant code in your question, not in the comments — trialNerror, Dec 01 '21 at 21:22
I have added the code, but your solution didn't work for me. my f I still empty when I call f from my action vector — Zain Ahmed, Dec 01 '21 at 21:37
This question is still missing a [mre]. We can not reproduce the problem you are describing, so any answer would be a guess. Maybe there's a problem with `setname`? Or with `setfunction`? Those are guesses about code we can not see. — Drew Dormann, Dec 01 '21 at 21:40
the setfunction could be problem here. but when i run the program I get an exception — Zain Ahmed, Dec 01 '21 at 22:28
setname and setfunction arent the issue here. the issue is accessing the function from the action vector, will give an empty function and results an exception despite I've defined it with my lambda expression. — Zain Ahmed, Dec 01 '21 at 22:32

score 0 · Answer 1 · answered Dec 01 '21 at 20:26

0

Your addAction method takes a reference

void addAction(Action&);

Then you call this with an rvalue

robot.addAction(Action("Up", [](int y) ->int{return y++; }));

You can not bind an rvalue to a non-const lvalue reference. Either make the method const ref or rvalue reference.

void addAction(const Action&);
void addAction(Action&&);

answered Dec 01 '21 at 20:26

Bako

313
1
15

This does explain why a C++ compiler can rightfully reject compiling this code, although I don't think this answers the question asked here. – Drew Dormann Dec 01 '21 at 20:57
You didn't answer my question, you only pointed out errors. I get it, but this isn't the answer I was looking for. – Zain Ahmed Dec 01 '21 at 21:36
@ZainAhmed Your question was: "How do I solve this?" And in the last code snippet I gave you the answer. – Bako Dec 01 '21 at 23:24
@Bako I did try your code snippet, but that did not work. I suggest give me more alternative answer. – Zain Ahmed Dec 02 '21 at 16:37

score -2 · Accepted Answer · answered Dec 01 '21 at 20:23

-2

You define

Learning::addAction(Action&)

with an action reference as argument. This means that you have to ensure the lifetime of the referenced object, and not let it get destroyed by going out of scope. But then you call it on an rvalue

// Action(...) immediately goes out of scope
robot.addAction(Action("Up", [](int y) ->int{return y++; }));

Therefore the references you put in your vectors are all invalid : they point toward long gone objects. You need to take ownership of them if you want it to work. This means :

class Learning
{  
  public:
    void addAction(Action&&);
  private:
    std::vector<Action> actions;
};

// rvalue reference, so the vector can take ownership of the expiring object
void Learning::addAction(Action&& act)
{
  actions.push_back(std::move(act));
}

answered Dec 01 '21 at 20:23

trialNerror

3,255
7
18

1

The OP's code shouldn't even compile, since a temporary object can't bind to a non-const lvalue reference like `Action&`, only to a const lvalue reference like `const Action&` or to an rvalue reference like `Action&&`. In any case, the OP's `addAction()` is calling the overload of `vector::push_back()` that takes in an lvalue reference, so it will *make a copy* of the passed `Action` object, so the lifetime of the original `Action` is not a factor here. But you are correct that using move semantics would be a better design. – Remy Lebeau Dec 01 '21 at 20:24
@RemyLebeau I disagree, an rvalue like Action(...) cannot be bound to an lvalue reference like Action&. The fact that it calls push_back internally does not change anything to the way addAction is called (erroneously) Edit : I agree with your edit – trialNerror Dec 01 '21 at 20:29
"*an rvalue like Action(...) cannot be bound to an lvalue reference like Action&.*" - that is exactly what I said in my previous comment. As for `push_back()`, inside of `addAction()`, the `act` parameter is an lvalue, even if passed in an rvalue, so `push_back(act)` will call the lvalue version that makes a copy, not the rvalue version that performs a move, that requires `push_back(std::move(act))`, as you showed. – Remy Lebeau Dec 01 '21 at 20:36
@RemyLebeau Yes I now agree 100% with you, and had written my comment before seeing that you had updated yours. I'll delete this answer which is plain wrong – trialNerror Dec 01 '21 at 20:39
Your solution is correct, it is just your explanation of the problem that is wrong. – Remy Lebeau Dec 01 '21 at 20:40
Um I got the problem because when i use std::move I get std::move returned I can't call the functions of the actions vector. every time I call actions[I].f(0), I get an exception. – Zain Ahmed Dec 01 '21 at 21:04
I tried with vs code, and it somehow worked for me. I guess when used class for my header file, that's where the problem starts... – Zain Ahmed Dec 02 '21 at 16:55

saving a lambda expression as parameter/variable inside a class to another class

2 Answers2