Run declaring transformers eagerly if possible, even if their inputs are deferred.

Run declaring transformers eagerly if possible, even if their inputs are deferred.

R=rnystrom@google.com
BUG=18270
Committed: https://code.google.com/p/dart/source/detail?r=35279

[Bob Nystrom, 2014-04-21 22:37:40]

https://codereview.chromium.org/243103003/diff/1/pkg/barback/lib/src/transfor...
File pkg/barback/lib/src/transform_node.dart (right):

https://codereview.chromium.org/243103003/diff/1/pkg/barback/lib/src/transfor...
pkg/barback/lib/src/transform_node.dart:213: (transformer is LazyTransformer ||
!primary.state.isAvailable)) {
This condition is pretty dense. How about breaking some of it out to a
well-named local variable?

I've read it several times, but I still don't get it. Why is it checking for "is
LazyTransformer" when the doc is about declaring ones?

[nweiz, 2014-04-21 22:52:08]

https://codereview.chromium.org/243103003/diff/1/pkg/barback/lib/src/transfor...
File pkg/barback/lib/src/transform_node.dart (right):

https://codereview.chromium.org/243103003/diff/1/pkg/barback/lib/src/transfor...
pkg/barback/lib/src/transform_node.dart:213: (transformer is LazyTransformer ||
!primary.state.isAvailable)) {
On 2014/04/21 22:37:40, Bob Nystrom wrote:
> This condition is pretty dense. How about breaking some of it out to a
> well-named local variable?

Done.

> I've read it several times, but I still don't get it. Why is it checking for
"is
> LazyTransformer" when the doc is about declaring ones?

Because this is the branch where we *don't* run apply.

[Bob Nystrom, 2014-04-22 18:22:42]

A couple of nits then LGTM.

https://codereview.chromium.org/243103003/diff/20001/pkg/barback/lib/src/tran...
File pkg/barback/lib/src/transform_node.dart (right):

https://codereview.chromium.org/243103003/diff/20001/pkg/barback/lib/src/tran...
pkg/barback/lib/src/transform_node.dart:209: // If [transformer] is declaring
and [primary] is available, we do want to
Oh, by "is declaring", you mean "is *only* declaring and *not* lazy". Lazy
transformers *are* declaring, so the "is! LazyTransformer" was deeply confusing.
Clarify the wording here.

"is declaring" -> "a non-lazy declaring one"

https://codereview.chromium.org/243103003/diff/20001/pkg/barback/lib/src/tran...
pkg/barback/lib/src/transform_node.dart:212: var canRunDeclaringEagerly =
How about just "canDeclareEagerly".

[nweiz, 2014-04-22 19:49:13]

https://codereview.chromium.org/243103003/diff/20001/pkg/barback/lib/src/tran...
File pkg/barback/lib/src/transform_node.dart (right):

https://codereview.chromium.org/243103003/diff/20001/pkg/barback/lib/src/tran...
pkg/barback/lib/src/transform_node.dart:209: // If [transformer] is declaring
and [primary] is available, we do want to
On 2014/04/22 18:22:42, Bob Nystrom wrote:
> Oh, by "is declaring", you mean "is *only* declaring and *not* lazy". Lazy
> transformers *are* declaring, so the "is! LazyTransformer" was deeply
confusing.
> Clarify the wording here.
> 
> "is declaring" -> "a non-lazy declaring one"

Done.

https://codereview.chromium.org/243103003/diff/20001/pkg/barback/lib/src/tran...
pkg/barback/lib/src/transform_node.dart:212: var canRunDeclaringEagerly =
On 2014/04/22 18:22:42, Bob Nystrom wrote:
> How about just "canDeclareEagerly".

That sounds like it means "can run declareOutputs eagerly", which isn't what's
going on here.

[nweiz, 2014-04-22 19:49:58]

Committed patchset #3 manually as r35279 (presubmit successful).