Handle Future result from Future.sync computation

Handle Future result from Future.sync computation

BUG= https://code.google.com/p/dart/issues/detail?id=13368
R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=27619

[floitsch, 2013-09-17 17:01:38]

I would prefer changing the parameter T from Future.value.
We accept futures there, so we shouldn't type it as T.

[kevmoo-old, 2013-09-17 17:21:41]

I updated as a compromise and realized I'm not a fan of this change, either.

Having the <T> param is a great baby-sitter.

Must we support (at least explicitly) new Future.value(Future) ?

https://codereview.chromium.org/23793006/diff/5001/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/23793006/diff/5001/sdk/lib/async/future.dart#...
sdk/lib/async/future.dart:131: return new Future.value(result);
After I did the update, I decided I don't like this either.

[floitsch, 2013-09-17 17:59:43]

https://codereview.chromium.org/23793006/diff/5001/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/23793006/diff/5001/sdk/lib/async/future.dart#...
sdk/lib/async/future.dart:131: return new Future.value(result);
On 2013/09/17 17:21:41, kevmoo wrote:
> After I did the update, I decided I don't like this either.

This Future<T> is correct. But the Future.value should not expect a T.
It means that the value will not be type-checked immediately but it will be
checked eventually (and we can help it by assigning it to T after the Future
check).

[kevmoo-old, 2013-09-18 12:59:31]

I'm going to miss the edit-time warnings for 'value', but knowing I'll get
yelled at in checked mode will do.

[floitsch, 2013-09-18 13:27:01]

LGTM.
thanks.

https://codereview.chromium.org/23793006/diff/13001/sdk/lib/async/future_impl...
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/23793006/diff/13001/sdk/lib/async/future_impl...
sdk/lib/async/future_impl.dart:146: // valid value types: T or Future<T>
Start comment with capital character and finish with ".".

https://codereview.chromium.org/23793006/diff/13001/sdk/lib/async/future_impl...
sdk/lib/async/future_impl.dart:325: if (value is Future &&
Maybe add (before this line):

// Assign to typed variables so we get earlier checks in checked mode.
if (value is Future) {
  Future<T> typedFuture = value;
} else {
  T typedValue = value;
}

[kevmoo-old, 2013-09-18 13:41:52]

https://codereview.chromium.org/23793006/diff/13001/sdk/lib/async/future_impl...
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/23793006/diff/13001/sdk/lib/async/future_impl...
sdk/lib/async/future_impl.dart:146: // valid value types: T or Future<T>
On 2013/09/18 13:27:01, floitsch wrote:
> Start comment with capital character and finish with ".".

Done.

https://codereview.chromium.org/23793006/diff/13001/sdk/lib/async/future_impl...
sdk/lib/async/future_impl.dart:325: if (value is Future &&
On 2013/09/18 13:27:01, floitsch wrote:
> Maybe add (before this line):
> 
> // Assign to typed variables so we get earlier checks in checked mode.
> if (value is Future) {
>   Future<T> typedFuture = value;
> } else {
>   T typedValue = value;
> }

Done.

[kevmoo-old, 2013-09-18 13:43:04]

Committed patchset #4 manually as r27619 (presubmit successful).