Re: [c++0x] <future>

[Jonathan Wakely <jwakely dot gcc at gmail dot com>]

On 20/05/2009, Benjamin Kosnik wrote:
>
>> This is as near as possible to the current working paper, n2857:
>> concepts, constexpr, new allocator support and explicit conversion
>> operators are missing.
>> There are some preprocessor conditions around missing features and g++
>> bugs, I'll remove them from the final version, this is really just to
>> let people try it out or comment, as I might be unable to work on it
>> again until next week.
>
> Thanks for the heads up. It looks like c++/40007 is being worked on, so
> I'm hoping those ifdef 0 parts goes away when this patch is checked in.
> Is that your plan as well?

Yep, the next version of the patch will remove all the ifdefs.  The
fix for 40007 isn't essential, so won't hold anything up if it doesn't
arrive soon.  I wanted to include everything, concepts, allocators and
all, in the first patch for completeness, but didn't intend to check
it in like that.

> Also, for the concept bits, I would rather see them commented vs. if
> defed. I think that is what the rest of the library is doing. You know,
> more like comment guidelines...
>
> ;)

Will do.

>> The implementation is a pretty simple one using a mutex and condition
>> variable (smarter versions using atomics to eliminate some locking are
>> possible.) Most of the code that doesn't depend on the type of
>> future/promise is in _Future_state_base for the shared state, or in
>> _Future_impl for the per-instance state.  That was done to reduce the
>> amount of code instantiated for every template specialisation.
>
> Sounds reasonable.

I'd misinterpreted the behaviour of packaged_task::operator() (and
written tests to verify that wrong behaviour) so I've fixed that.

I also realised that there's a potential for deadlock while setting
the value on the promise.  If the object being copied/moved into the
result checks the state of the associated future in the copy/move
operation it will deadlock, because the lock is held during the
copy/move.  That requires the result to be aware of the future that
will contain the result and such a cycle is probably unlikely, but
I'll see what I can do to avoid it.

I've also assumed the result is DefaultConstructable, I might need to
dynamically allocate it to avoid that.

>> As I've implemented it, you'll get a segfault if you use almost any
>> member function on a future/promise after a std::move.  That's
>> intentional: the only requirement is that they can be safely
>> destroyed, so I didn't add any checks for empty shared_ptr.
>
> Hmmm. This doesn't seem like a friend-making move. At least add a
> comment or note about this.

Will do, but maybe I made it sound unnecessarily evil.  It's the same as:

std::unique_ptr<int> p1(new int);
std::unique_ptr<int> p2 = std::move(p1);
*p1 = 0;  // segfault

Dereferencing a unique_ptr after a move *will* crash, and I don't
think that's unreasonable.  Using a promise after a move is (IMHO)
similar.  I'll do as unique_ptr does and add a _GLIBCXX_DEBUG_ASSERT.

>> The tests in this patch are a bit of a hodge-podge, some use
>> test01/test02 functions called by main, some have the test code
>> directly in main()... please let me know if I need to rework them.
>
> Please try to keep all test cases with minimal main functions, which
> call a test function or functor and return 0.

Sure, I'll rework them for the next version of the patch.

Thanks for the comments,

Jonathan