1

Precondition:

Here is a function:

typedef std::function<void (int)> Handler;
void g(const Handler& h) {
  h(100);
}

, and a class:

class A {
 public:
  void f0(int n) {
    std::cout << m + n << std::endl;
  }

  void f1() {
    ::g(std::bind(&A::f0, this, std::placeholders::_1));
  }

  int m;
};

And this will print two lines, '101' and '102':

int main() {
  A a1;
  a1.m = 1;
  a1.f1();

  A a2;
  a2.m = 2;
  a2.f1();

  return 0;
}

Now I realized A::f1() will be called very frequently,
so I modified it like this(new version):

  void A::f1() {
    static const Handler kHandler =
        std::bind(&A::f0, this, std::placeholders::_1);

    ::g(kHandler);
  }

My Questions:

Is it safe to bind this pointer to a local static variable?

Is there no functional difference between two versions?

Can I expect the new version will really gain some performance benefit?
(I suspect my compiler(MSVC) will optimize it by itself,
so I may not need to optimize it by myself).

EDITED ----------

I run the new version and realized that the result is not the same as the original one.
It prints two lines, '101' and '101' again(not '102').
Poor question, sorry for all.

EDITED 2 ----------

Please refer to my new question which I might truly intend:
Binding member function to a member variable

Community
  • 1
  • 1
ALittleDiff
  • 1,191
  • 1
  • 7
  • 24
  • 2
    It is dangerous if `A::f1` is called for different instances, since the first one gets captured in the static, and all the other instances will use the copy bound to the first instance (which may no longer exist). – Raymond Chen Oct 24 '14 at 03:08
  • @RaymondChen: You are right. I run this code and cleared this issue. – ALittleDiff Oct 24 '14 at 03:25
  • I just realized that this is exactly what happens in your sample code, so you would have noticed the problem if you had actually tried it. – Raymond Chen Oct 24 '14 at 04:04

2 Answers2

1

No, this is not safe (nor works as intended). The static variable is shared among all instances to A, and you bind this in this static function object kHandler when calling f1 for the first time. So the bound parameter is always equal to the instance on which you called f1 first, i.e. in your case a1.

It's basically the same with this function:

int f(int a) {
    static int b = a;
    return b;
}

Call this function multiple times, and you will always get the value of the first call. (Demo)

Alternatives:

  • You could, if you can live with a space overhead, use a member variable for the bound function, though. I guess implementing this is straight-forward.

  • A non-thread-safe alternative (I'd not recommend using this!) could be to store the "this" pointer in a static member variable ("that") and make f0 static and use "that" instead of "this":

    class A {
        static A * that = nullptr;
    
    public:
        static void f0(int n) {
            assert(that);
            std::cout << that->m + n << std::endl;
        }
    
        void f1() {
            assert(!that);
            that = this;
            ::g(&A::f0);
            that = nullptr;
        }
    
        int m;
    };
    
leemes
  • 44,967
  • 21
  • 135
  • 183
0

Raymond Chen's comment is Correct - by using static you're only ever creating one instance of kHandler, and if the instance of A associated with that first call ever dies, then the bound "this" pointer will be dead.

I recommend removing static:

void A::f1() {
   const Handler kHandler =
       std::bind(&A::f0, this, std::placeholders::_1);

   ::g(kHandler);
 }

This is safe because kHandler will exist across the lifetime of the g call.

Brad
  • 3,190
  • 1
  • 22
  • 36
  • Well, that's no advantage whatsoever over the original code (before the optimization approach). – leemes Oct 24 '14 at 03:21
  • @leemes: I agree with you, but I just wanted to know if a member function can be bound to a local static variable or not, and I realized it can't. Anyway, thank you for your answer, too. (*) – ALittleDiff Oct 24 '14 at 03:41