[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

A few small comments...



================
Comment at: libcxx/include/functional:1821
 {
-    if ((void *)__f_ == &__buf_)
-        __f_->destroy();
-    else if (__f_)
-        __f_->destroy_deallocate();
-    __f_ = 0;
+    function::operator=(nullptr);
     if (__f.__f_ == 0)
----------------
Couldn't this be more clearly written as `*this = nullptr;` ?


================
Comment at: libcxx/include/functional:1822
+    function::operator=(nullptr);
     if (__f.__f_ == 0)
         __f_ = 0;
----------------
At this point `__f_ == 0` (because we just set it to 0).
We probably don't need to do that again.



================
Comment at: libcxx/include/functional:1843
     __f_ = 0;
+    if ((void *)__t == &__buf_)
+        __t->destroy();
----------------
I see this dance 4x in `<functional_03>` and once here. Did you give any thought to making it a function of `__base`?  Maybe call it `__clear`?

Then line #1821 can be written as `__clear();`




https://reviews.llvm.org/D34331