dexonsmith added a comment. In https://reviews.llvm.org/D34331#802747, @EricWF wrote:
> @dexonsmith Mind if I hijack this and check in your changes to `<functional>` > with my tests? Not at all. In https://reviews.llvm.org/D34331#802821, @EricWF wrote: > @dexonsmith I'm not sure it's sane to allow reentrant behavior. Could you > explain why you think it is? It didn't seem sane to me at first either, despite this being supported by `std::unique_ptr` and `std::shared_ptr`. I found our user fairly convincing, though: - They had an underlying reference counted object, so only the destruction of the last copy of A nulled out the pointer (mimicked that with the `bool cancel`). - They had a callback function intended to be called once, and dropping that function on cancellation (mimicked that with a global variable). The cancel operation nulled out a `std::function`, but destroying the objects captured in the lambda in that std::function also separately decided to perform a cancel, which should have been OK. The cancel function just contained `callback = nullptr`. In https://reviews.llvm.org/D34331#802821, @EricWF wrote: > Should the copy assignment operator allow reentrancy as well? I'm not sure; what do you think? ================ Comment at: libcxx/test/libcxx/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/move_reentrant.pass.cpp:1 +//===----------------------------------------------------------------------===// +// ---------------- EricWF wrote: > It seems like this test is testing behavior that should be required by the > standard, right? > > If so it should live under `test/std`. Yes, that likely is a better place. ================ Comment at: libcxx/test/libcxx/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/move_reentrant.pass.cpp:25 + ~A() { + asm(""); + if (cancel) ---------------- EricWF wrote: > Is `asm("")` just to prevent optimizations? If so please use `DoNotOptimize` > from `test_macros.h`. Sounds fine to me. https://reviews.llvm.org/D34331 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
