Don't wait unnecessarily.

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

[floitsch, 2011-10-27 16:16:54]

LGTM.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
File tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart
(right):

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:252:
class ProxySet<T extends Proxy> implements Collection<T> {
why not Set<T> ?

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:260: void
add (T t) {
remove space.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:261: for
(T x in _set)
use {}

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:263:
return;
either put "return" on the same line, or put {} around.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:264: if
(t.hasValue())
{}

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:265: for
(T x in _set)
{}

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:266: if
(x.hasValue() && x == t)
{} or same line.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:276: for
(int n = 0; n < _set.length; ++n)
{}

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:277: if
(_set[n].hasValue() && _set[n] == t)
{}

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:292:
ProxySet<T> filter(bool f(T element))
convention is that => is only allowed on the same line.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:324: if
(t != null)
{} or same line.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:324: if
(t != null)
t !== null

[Ben Laurie (Google), 2011-10-27 16:34:57]

BTW, FullyIsolated now passes on dartc but hangs on VM - any idea why?

I will convert to Set<T> tomorrow.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
File tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart
(right):

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:252:
class ProxySet<T extends Proxy> implements Collection<T> {
On 2011/10/27 16:16:54, floitsch wrote:
> why not Set<T> ?

Lazy :-)

I will fix.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:261: for
(T x in _set)
On 2011/10/27 16:16:54, floitsch wrote:
> use {}

Sigh.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:263:
return;
On 2011/10/27 16:16:54, floitsch wrote:
> either put "return" on the same line, or put {} around.

Really? Is that official? I _hate_ that style (both of them).

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:292:
ProxySet<T> filter(bool f(T element))
On 2011/10/27 16:16:54, floitsch wrote:
> convention is that => is only allowed on the same line.

Done.

http://codereview.chromium.org/8403040/diff/5001/tests/stub-generator/src/Min...
tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart:324: if
(t != null)
On 2011/10/27 16:16:54, floitsch wrote:
> t !== null

Really? I've been explcitly told === is not needed for null.

[floitsch, 2011-11-02 14:34:27]

LGTM with comments.

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
File
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java
(right):

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java:110:
// [x] is a promise for a proxy.
Hehe. This is Java. No [x] here :)

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java:342:
p("return new Promise<" + type + ">.fromValue(new " + type + "Impl(new
PromiseProxy<SendPort>(new PromiseProxy<SendPort>(");
This is just way too much without comments...: a promise with value of a
promiseForProxyImpl of a promiseproxy of a promiseproxy...

http://codereview.chromium.org/8403040/diff/15001/corelib/src/implementation/...
File corelib/src/implementation/promise_implementation.dart (right):

http://codereview.chromium.org/8403040/diff/15001/corelib/src/implementation/...
corelib/src/implementation/promise_implementation.dart:335: Promise
_marshal(List message, process(List marshalled)) {
indentation?

http://codereview.chromium.org/8403040/diff/15001/tests/stub-generator/testcf...
File tests/stub-generator/testcfg.py (right):

http://codereview.chromium.org/8403040/diff/15001/tests/stub-generator/testcf...
tests/stub-generator/testcfg.py:42: def IsFailureOutput(self, output):
Are you sure this is necessary?
Do all components now support 'print' in testing mode?

[Ben Laurie (Google), 2011-11-02 14:45:44]

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
File
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java
(right):

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java:110:
// [x] is a promise for a proxy.
On 2011/11/02 14:34:27, floitsch wrote:
> Hehe. This is Java. No [x] here :)

Comment was pointless anyway, deleted.

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java:342:
p("return new Promise<" + type + ">.fromValue(new " + type + "Impl(new
PromiseProxy<SendPort>(new PromiseProxy<SendPort>(");
On 2011/11/02 14:34:27, floitsch wrote:
> This is just way too much without comments...: a promise with value of a
> promiseForProxyImpl of a promiseproxy of a promiseproxy...

Done.

http://codereview.chromium.org/8403040/diff/15001/corelib/src/implementation/...
File corelib/src/implementation/promise_implementation.dart (right):

http://codereview.chromium.org/8403040/diff/15001/corelib/src/implementation/...
corelib/src/implementation/promise_implementation.dart:335: Promise
_marshal(List message, process(List marshalled)) {
On 2011/11/02 14:34:27, floitsch wrote:
> indentation?

Done.

http://codereview.chromium.org/8403040/diff/15001/tests/stub-generator/testcf...
File tests/stub-generator/testcfg.py (right):

http://codereview.chromium.org/8403040/diff/15001/tests/stub-generator/testcf...
tests/stub-generator/testcfg.py:42: def IsFailureOutput(self, output):
On 2011/11/02 14:34:27, floitsch wrote:
> Are you sure this is necessary?

Yes, without it the test can exit early and appear to work even though it does
not.

> Do all components now support 'print' in testing mode?

I don't know, but we need _some_ equivalent mechanism or we cannot, in general,
test asynchronous code.

Being able to set an exit code would be another example of a method that should
work.

[floitsch, 2011-11-02 16:04:17]

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
File
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java
(right):

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java:342:
p("return new Promise<" + type + ">.fromValue(new " + type + "Impl(new
PromiseProxy<SendPort>(new PromiseProxy<SendPort>(");
On 2011/11/02 14:45:44, Ben Laurie (Google) wrote:
> On 2011/11/02 14:34:27, floitsch wrote:
> > This is just way too much without comments...: a promise with value of a
> > promiseForProxyImpl of a promiseproxy of a promiseproxy...
> 
> Done.
Can't see them.

http://codereview.chromium.org/8403040/diff/15001/tests/stub-generator/testcf...
File tests/stub-generator/testcfg.py (right):

http://codereview.chromium.org/8403040/diff/15001/tests/stub-generator/testcf...
tests/stub-generator/testcfg.py:42: def IsFailureOutput(self, output):
On 2011/11/02 14:45:44, Ben Laurie (Google) wrote:
> On 2011/11/02 14:34:27, floitsch wrote:
> > Are you sure this is necessary?
> 
> Yes, without it the test can exit early and appear to work even though it does
> not.
The testing-infrastructure should make sure this is not the case. If not, please
give us an example so that we can fix the test-framework. (This is the whole
reason why you have to write expect.succeeded().)
> 
> > Do all components now support 'print' in testing mode?
> 
> I don't know, but we need _some_ equivalent mechanism or we cannot, in
general,
> test asynchronous code.
> 
> Being able to set an exit code would be another example of a method that
should
> work.

[Ben Laurie (Google), 2011-11-02 21:09:34]

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
File
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java
(right):

http://codereview.chromium.org/8403040/diff/15001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/backend/isolate/DartIsolateStubGenerator.java:342:
p("return new Promise<" + type + ">.fromValue(new " + type + "Impl(new
PromiseProxy<SendPort>(new PromiseProxy<SendPort>(");
On 2011/11/02 16:04:17, floitsch wrote:
> On 2011/11/02 14:45:44, Ben Laurie (Google) wrote:
> > On 2011/11/02 14:34:27, floitsch wrote:
> > > This is just way too much without comments...: a promise with value of a
> > > promiseForProxyImpl of a promiseproxy of a promiseproxy...
> > 
> > Done.
> Can't see them.

I have no idea why, but uploaded now.

http://codereview.chromium.org/8403040/diff/15001/tests/stub-generator/testcf...
File tests/stub-generator/testcfg.py (right):

http://codereview.chromium.org/8403040/diff/15001/tests/stub-generator/testcf...
tests/stub-generator/testcfg.py:42: def IsFailureOutput(self, output):
On 2011/11/02 16:04:17, floitsch wrote:
> On 2011/11/02 14:45:44, Ben Laurie (Google) wrote:
> > On 2011/11/02 14:34:27, floitsch wrote:
> > > Are you sure this is necessary?
> > 
> > Yes, without it the test can exit early and appear to work even though it
does
> > not.
> The testing-infrastructure should make sure this is not the case. If not,
please
> give us an example so that we can fix the test-framework. (This is the whole
> reason why you have to write expect.succeeded().)

How? AFAICS it does not achieve that: for example, if I return with outstanding
promises, the result is never tested.

> > 
> > > Do all components now support 'print' in testing mode?
> > 
> > I don't know, but we need _some_ equivalent mechanism or we cannot, in
> general,
> > test asynchronous code.
> > 
> > Being able to set an exit code would be another example of a method that
> should
> > work.
>