vsapsai added inline comments.
================
Comment at: libcxx/include/functional:1722-1733
if (__f.__f_ == 0)
__f_ = 0;
else if ((void *)__f.__f_ == &__f.__buf_)
{
__f_ = __as_base(&__buf_);
__f.__f_->__clone(__f_);
}
----------------
Here is the similar code I've mentioned.
================
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)
----------------
mclow.lists wrote:
> Couldn't this be more clearly written as `*this = nullptr;` ?
Personally I prefer to call operator explicitly but checking the code shows it
isn't very common. Will change.
================
Comment at: libcxx/include/functional:1822
+ function::operator=(nullptr);
if (__f.__f_ == 0)
__f_ = 0;
----------------
mclow.lists wrote:
> At this point `__f_ == 0` (because we just set it to 0).
> We probably don't need to do that again.
>
This code is the same as in `function(function&& __f)`. Do you think there is
enough value to deviate from that implementation?
================
Comment at: libcxx/include/functional:1843
__f_ = 0;
+ if ((void *)__t == &__buf_)
+ __t->destroy();
----------------
mclow.lists wrote:
> 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();`
>
>
Haven't really thought about that. Not sure it can be easily done. All `__base`
classes are separate templates and don't have a common ancestor. Don't want to
introduce macros as it doesn't look like libc++ style. You achieve the most
consistency with current code by copy-pasting and repeating the same dance a
few times.
https://reviews.llvm.org/D34331
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
