C++ Can't figure out my class-specific function?

Tsaukpaetra

no known conversion for argument 2 from 'void (Module_NET::*)(WebDuino&, WebDuino::ConnectionType, char*, bool)' to 'void (*)(WebDuino&, WebDuino::ConnectionType, char*, bool)'

So, I'm trying to pass a class's function to the function in a sub-class which will callback on that function like so:

webserver.addCommand("fwupdate", &Module_NET::ws_fwupdate);

Library code that actually defines the addCommand function here, and that's just this:

 void addCommand(const char *verb, Command *cmd);

How do I fix it so it accepts functions from the Module_NET instance?

dkf

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

How do I fix it so it accepts functions from the Module_NET instance?

The problem is that it wants a pointer to a function (or a static method), not a pointer to an instance's method. This matters because those are a different size (a pointer to an instance method is actually internally a pointer to the method and a pointer to the instance on which that method is to be called) and they have different calling conventions.

So… how do you plan to smuggle this through the static method or function?

Tsaukpaetra

@dkf said in C++ Can't figure out my class-specific function?:

So… how do you plan to smuggle this through the static method or function?

I don't, nothing here is intended to be static, just that this library happens to try and make things easier to type by accepting only static functions here (I guess?).

Neither the webserver or the function I want called are static.

I just need to figure out how to get the library's class to realize that.

Gąska

@dkf said in C++ Can't figure out my class-specific function?:

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

How do I fix it so it accepts functions from the Module_NET instance?

The problem is that it wants a pointer to a function (or a static method), not a pointer to an instance's method. This matters because those are a different size (a pointer to an instance method is actually internally a pointer to the method and a pointer to the instance on which that method is to be called) and they have different calling conventions.

No, a pointer to instance method is internally still just a single pointer to a function. You provide an instance to the pointer at call site, and the syntax for that is as awful as everything else related to function pointers. But yes, the main problem is still that the pointer to function and pointer to method are incompatible types.

@Tsaukpaetra instead of using pointers, store callbacks as std::function. It does all the necessary conversions and wrapping automatically. You can then use std::bind to, well, bind a method to the instance. You can bind the other parameters too.

Tsaukpaetra

@Gąska said in C++ Can't figure out my class-specific function?:

store callbacks as std::function.

I'm not entirely confident that the C++ version level available to me has this. I'm reading up on this now, trying to figure out all the places where I'll need to fucker up the library to get that to happen...

Tsaukpaetra

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

I'm not entirely confident that the C++ version level has this.

Okay, it seems it does.

Now to figure out how to fix the code. I replaced the pointer to typedef of the function like so, from

  typedef void Command(WebDuino &server, ConnectionType type,
                       char *url_tail, bool tail_complete);
  struct CommandMap
  {
    const char *verb;
    Command *cmd;
  } m_commands[WEBDUINO_COMMANDS_COUNT];

to

  typedef void Command(WebDuino &server, ConnectionType type,
                       char *url_tail, bool tail_complete);
  struct CommandMap
  {
    const char *verb;
    std::function<Command> *cmd;
  } m_commands[WEBDUINO_COMMANDS_COUNT];

Since it can no longer use bare static function pointers (and I'm not even entirely certain I did the change above correctly, I get the following error (because presumably I haven't cast the thing? Or something?)

error: cannot convert 'void (*)(WebDuino&, WebDuino::ConnectionType, char*, bool)' to 'std::function<void(WebDuino&, WebDuino::ConnectionType, char*, bool)>*' in assignment

topspin

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

and I'm not even entirely certain I did the change above correctly

No, you didn't.
The Command typedef is a typedef for a function (pointer). You changed the CommandMap struct from containing such function pointers to pointers to std::function. That's an additional level of indirection. It should probably just be std::function<Command> cmd;.

Although I have a headache and function typedef syntax is an abomination so I didn't read this too carefully.

Tsaukpaetra

@topspin said in C++ Can't figure out my class-specific function?:

It should probably just be std::function<Command> cmd;.

Yeah, that seems to have helped, all compiles OK with the static functions again and no other changes (which I hope is a sign that the magical compiler is doing all the behind-scene needfuls).

Back to trying to add the instance-specific functions and...

error: no matching function for call to 'WebDuino::addCommand(const char [9], void (Module_NET::*)(WebDuino&, WebDuino::ConnectionType, char*, bool))'
no known conversion for argument 2 from 'void (Module_NET::*)(WebDuino&, WebDuino::ConnectionType, char*, bool)' to 'std::function<void(WebDuino&, WebDuino::ConnectionType, char*, bool)>'

Ugh...

Gąska

@Tsaukpaetra std::function treats methods like regular functions with added one parameter in front (for this pointer). So you are trying to assign a 5-arg function to a 4-arg std::function. You can do that if you first assign a value to the first argument using std::bind:

using namespace std::placeholders;

cmd = std::bind(&Module_NET::ws_fwupdate, <INSERT INSTANCE HERE>, _1, _2, _3, _4);

E.g.

cmd = std::bind(&Module_NET::ws_fwupdate, this, _1, _2, _3, _4);

Edit: fixed syntax.

topspin

@Tsaukpaetra now you need to make a callable that basically wraps (or "smuggles" as @dkf said above) the this pointer of the Module_NET object (You do have such an object, right?). As above, you can use a std::bind, which quickly gets unreadable, or use a lambda.

Completely untested:
Either

Module_NET* mod;  // wherever you have this from

using namespace std::placeholders;
webserver.addCommand("fwupdate",
    std::bind(&Module_NET::ws_fwupdate, mod, _1, _2, _3, _4));

or

Module_NET* mod;  // wherever you have this from

webserver.addCommand("fwupdate",
  [mod](WebDuino &server, ConnectionType type, char *url_tail, bool tail_complete) {
        mod->ws_fwupdate(server, type, url_tail, tail_complete);
    });

E:

Tsaukpaetra

@topspin said in C++ Can't figure out my class-specific function?:

@Tsaukpaetra now you need to make a callable that basically wraps (or "smuggles" as @dkf said above) the this pointer of the Module_NET object (You do have such an object, right?).

I was trying to make a template function that should do this, but I'm lost in these very tall weeds.

  // add a new command to be run at the URL specified by verb
  void addCommand(const char *verb, std::function<Command> cmd);
  //Now do the same but for instance member functions
  template <typename X>
  void addCommand(const char *verb, std::function<X::<Command>> cmd); //???

This so that the addCommand syntax does not change. Or, if it does change, I just need to slap a this to addCommand.

Gąska

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

@topspin said in C++ Can't figure out my class-specific function?:

@Tsaukpaetra now you need to make a callable that basically wraps (or "smuggles" as @dkf said above) the this pointer of the Module_NET object (You do have such an object, right?).

I was trying to make a template function that should do this

Don't. Just don't. Hold your nose, write that std::bind line and be done with it. The only thing worse than C++ syntax is trying to figure out how to make it nicer.

Tsaukpaetra

@Gąska said in C++ Can't figure out my class-specific function?:

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

@topspin said in C++ Can't figure out my class-specific function?:

@Tsaukpaetra now you need to make a callable that basically wraps (or "smuggles" as @dkf said above) the this pointer of the Module_NET object (You do have such an object, right?).

I was trying to make a template function that should do this

Don't. Just don't. Hold your nose, write that std::bind line and be done with it. The only thing worse than C++ syntax is trying to figure out how to make it nicer.

But the bind command does the opposite of what I need to do. I don't need to create a function callback to call the addCommand function with arbitrary parameters, I need a function callback that calls the member function given with the specific parameters....

topspin

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

But the bind command does the opposite of what I need to do.

The bind command (and the lambda alternative) allow you to do exactly what you tried to do in the OP.

Tsaukpaetra

@topspin said in C++ Can't figure out my class-specific function?:

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

But the bind command does the opposite of what I need to do.

The bind command (and the lambda alternative) allow you to do exactly what you tried to do in the OP.

Yours seems to. @Gąska's does not...?

I'm just confused why making these lambda functions is easier than making a template that makes the lambda function for me, so

addCommand("blah",&myStaticFunction);

becomes

addCommand("blah",this,&MyClass::MyMemberFunction);

?

Gąska

@Tsaukpaetra I messed up the method name in my example, which may have confused you about what std::bind does. I edited the post but maybe you haven't refreshed properly. Anyway, it's all really simple, once you get through the epicly opaque syntax.

You have a variable cmd that is an std::function taking 4 arguments. You also have a class Module_NET with a method ws_fwupdate taking the same 4 arguments. You want to assign ws_fwupdate to cmd.

But as far as std::function is concerned, ws_fwupdate is a 5-argument function where the first argument is of type Module_NET*. And that's why you get an error.

To solve this, you have to somehow turn the 5-argument function into 4-argument function. This is exactly what std::bind does - it takes a function and some arguments and turns it into a function with less arguments. Specifically in this case, it takes a method and an instance to create a callback that only needs 4 arguments to call and always calls that method with that instance. Now you can assign it to cmd and it will work properly.

Just remember to keep the module alive. If you feel lazy, std::bind has appropriate overloads that make it work with smart pointers too.

topspin

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

@topspin said in C++ Can't figure out my class-specific function?:

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

But the bind command does the opposite of what I need to do.

The bind command (and the lambda alternative) allow you to do exactly what you tried to do in the OP.

Yours seems to. @Gąska's does not...?

He left out the following line where you'd call addCommand("bla", cmd);. The crucial part was in the definition of cmd (although you need to put an auto in front of it to declare the variable there).

I'm just confused why making these lambda functions is easier than making a template that makes the lambda function for me, so
addCommand("blah",&myStaticFunction);
becomes
addCommand("blah",this,&MyClass::MyMemberFunction);
?

The syntax for it is probably awful. Member function pointers are weird, I don't remember off the top of my head if it works exactly like I think it should, works differently, or doesn't work at all. In any case, I'd need to actually try it with a compiler to write that function.
But you're right, if you had such a template the invocation you wrote would be easier. (Although it wouldn't necessarily be called this unless you're already inside of a member function of MyClass. In general it's a pointer to the MyClass instance you want to invoke it on)

Gąska

@topspin said in C++ Can't figure out my class-specific function?:

But you're right, if you had such a template the invocation you wrote would be easier.

Don't encourage him! I really don't feel like giving a lecture about perfect forwarding and the rest of template bullshit that's absolutely necessary to make it work. He's not ready for it.

Seriously, just stick with std::bind.

Tsaukpaetra

@topspin said in C++ Can't figure out my class-specific function?:

The syntax for it is probably awful.

I'm getting pretty close actually. Once I get it to match the template I think I should be golden. So far I've gotten it down to:

cannot convert '&Module_NET::ws_command' (type 'void (Module_NET::*)(WebDuino&, WebDuino::ConnectionType, char*, bool)') to type 'void (Module_NET::*)(void (*)(WebDuino&, WebDuino::ConnectionType, char*, bool))'

   webserver.addCommand<Module_NET>("command",this, &Module_NET::ws_command);

Tsaukpaetra

@Gąska said in C++ Can't figure out my class-specific function?:

@topspin said in C++ Can't figure out my class-specific function?:

But you're right, if you had such a template the invocation you wrote would be easier.

Don't encourage him! I really don't feel like giving a lecture about perfect forwarding and the rest of template bullshit that's absolutely necessary to make it work. He's not ready for it.

Seriously, just stick with std::bind.

It's not like I'm trying to make it work with arbitrary args and not-predefined types. Nothing magical other than doing the bind thing automatically without copy-pasta on the front.

topspin

@Gąska said in C++ Can't figure out my class-specific function?:

@topspin said in C++ Can't figure out my class-specific function?:

But you're right, if you had such a template the invocation you wrote would be easier.

Don't encourage him! I really don't feel like giving a lecture about perfect forwarding and the rest of template bullshit that's absolutely necessary to make it work. He's not ready for it.

Seriously, just stick with std::bind.

You don't need to make it generic in the function's arguments, that can be fixed to be (WebDuino &server, ConnectionType type, char *url_tail, bool tail_complete). So you don't need perfect forwarding. Unless I'm missing something?

Gąska

@topspin at the least, you have to forward this into std::bind. And it needs to be an rvalue reference or whatever it's called or else you risk making additional copies and maybe even making types incompatible in some but not all cases. And remember that rvalue references behave differently inside templates than in regular code. And probably ten other caveats that I don't remember right now because I haven't written C++ in years. Basically, it's best to avoid writing template wrappers around other template functions at all, unless you feel particularly masochistic and really want to go down this particular rabbit hole.

topspin

@Gąska said in C++ Can't figure out my class-specific function?:

@topspin at the least, you have to forward this into std::bind. And it needs to be an rvalue reference

No, this is a pointer (even though it should be a reference if the language was sane), so you can just copy it. Lifetime issues are also identical for the wrapper as for the immediate call.

Tsaukpaetra

@topspin said in C++ Can't figure out my class-specific function?:

So you don't need perfect forwarding. Unless I'm missing something?

Yeah, I'm not interested in doing that.

And I think I've met with some success (at least, it compiles)!

  // add a new command to be run at the URL specified by verb
  void addCommand(const char *verb, std::function<Command> cmd);
  //Now do the same but for instance member functions
template <typename X>
  void addCommand(const char *verb, X* obj,void (X::* func)(WebDuino &server, ConnectionType type, char *url_tail, bool tail_complete)){
    addCommand(verb, [obj, func](WebDuino &server, WebDuino::ConnectionType type, char *url_tail, bool tail_complete) {
        (obj->*func)(server, type, url_tail, tail_complete);
    });
  }

Now to see what the fuck happens when I try to actually run it...

Gąska

@topspin okay, if he's gonna hardcode that it's always a raw pointer and never anything else, then yes, it might work.

Gąska

@Tsaukpaetra oh, you're making a lambda. Okay, that's much less of an issue.

Still, it'll be worth to read up on std::bind at one point. It's very easy and extremely useful.

Gąska

I should've learned by now that "edit to add" isn't an effective way of communicating with @Tsaukpaetra.

Tsaukpaetra

@Gąska said in C++ Can't figure out my class-specific function?:

I should've learned by now that "edit to add" isn't an effective way of communicating with @Tsaukpaetra.

Not once the message has been read, no. Our NodeBB hates refreshing posts when you're still in the thread.

topspin

I think it should be as simple as this:


    template <typename T>
    void addCommand(const char* verb, T* instance, void (T::* memfn)(WebDuino &server,
          ConnectionType type, char *url_tail, bool tail_complete))
    {
        using namespace std::placeholders;
        addCommand(verb, std::bind(memfn, instance, _1, _2, _3, _4));
    }

But obviously I haven't tested it. Beware, @Gąska has warned you (and cursed me).

ObjectMike

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

@Gąska said in C++ Can't figure out my class-specific function?:

I should've learned by now that "edit to add" isn't an effective way of communicating with @Tsaukpaetra.

Not once the message has been read, no. Our NodeBB hates refreshing posts when you're still in the thread.

I've been noticing recently edits showing up real time. Or at least my browser showing a refresh that only seems to happen soon after a new post appears.

Gąska

@topspin I'd still go with lambda. Less caveats to keep track of. The semantics of template functions calling other template functions make up like half of the entire C++ standard.

Tsaukpaetra

@Gąska said in C++ Can't figure out my class-specific function?:

@topspin I'd still go with lambda. Less caveats to keep track of. The semantics of template functions calling other template functions make up like half of the entire C++ standard.

So a really fast smoke test seems to have worked.

Now to get on with actually writing the code I wanted to be writing three hours ago...

topspin

@Gąska Well, I originally had in a "which I personally prefer" before the lambda alternative I gave to std::bind. Because I hate bind and never use it. But seeing that the lambda is quite more verbose, I figured I'd go with it.

topspin

@Tsaukpaetra said in C++ Can't figure out my class-specific function?:

And I think I've met with some success (at least, it compiles)!

Yes, I think that's basically what I came up with too.
So, 'd once again, by the questioner.

Tsaukpaetra

@topspin said in C++ Can't figure out my class-specific function?:

the lambda is quite more verbose,

The verbosity is fine if it's done only once. Once you start copy-pasta the brevity quickly becomes more preferred.

This way there's the best of both worlds, I get a pretty verbose code snip that happens once if I really want to go into the library to determine how something works under the hood, and on my code side it's just a "well, include the object pointer first if it's not a static function".

dkf

@Gąska said in C++ Can't figure out my class-specific function?:

No, a pointer to instance method is internally still just a single pointer to a function. You provide an instance to the pointer at call site, and the syntax for that is as awful as everything else related to function pointers.

Really? What's the flaming point of that?! It's all the pain and exactly none of the gain. My god, the C++ committee must've really been handing round the funny juice for a few days straight before when they let that in.

topspin

@dkf you can separate the pointer to the member function from the instance pointer and use it on different instances. If you have a whole bunch of instances to apply it to, you only need one pointer.
Also, it’s just a thin, leaking abstraction over its C implementation, treating the this parameter as if it was explicit. If you want a bound pointer, you can build it from that, but not the other way around.