Issue 294123009: Make lazy and declaring transformers work with $include/$exclude.

Issue 294123009: Make lazy and declaring transformers work with $include/$exclude. (Closed)

Created:
6 years, 7 months ago by nweiz

Modified:
6 years, 6 months ago

Reviewers:
Bob Nystrom, Alan Knight

CC:
reviews_dartlang.org, Bob Nystrom

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Make lazy and declaring transformers work with $include/$exclude. RELNOTE=Transformer laziness/declaringness is preserved when using $include/$exclude. R=alanknight@google.com Committed: https://code.google.com/p/dart/source/detail?r=36460

Patch Set 1 #

Patch Set 2 : analyzer fixes #

Total comments: 4

Created: 6 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+87 lines, -177 lines)			Patch
M	sdk/lib/_internal/pub/lib/src/barback/excluding_transformer.dart	View	1	2 chunks	+26 lines, -1 line	4 comments	Download
D	sdk/lib/_internal/pub/test/serve/supports_user_defined_declaring_transformers.dart	View		1 chunk	+0 lines, -96 lines	0 comments	Download
A +	sdk/lib/_internal/pub/test/serve/supports_user_defined_declaring_transformers_test.dart	View		4 chunks	+5 lines, -30 lines	0 comments	Download
M	sdk/lib/_internal/pub/test/serve/supports_user_defined_lazy_transformers_test.dart	View		2 chunks	+1 line, -26 lines	0 comments	Download
M	sdk/lib/_internal/pub/test/serve/utils.dart	View		1 chunk	+26 lines, -0 lines	0 comments	Download
M	sdk/lib/_internal/pub/test/transformer/exclusion/works_on_dart2js_test.dart	View		2 chunks	+10 lines, -1 line	0 comments	Download
A +	sdk/lib/_internal/pub/test/transformer/exclusion/works_on_lazy_transformer_test.dart	View		1 chunk	+19 lines, -23 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

nweiz

Committed patchset #2 manually as r36460 (presubmit successful).

6 years, 7 months ago (2014-05-21 23:27:35 UTC) #3

Bob Nystrom

One request (for a later patch), one question, then LGTM. https://codereview.chromium.org/294123009/diff/20001/sdk/lib/_internal/pub/lib/src/barback/excluding_transformer.dart File sdk/lib/_internal/pub/lib/src/barback/excluding_transformer.dart (right): https://codereview.chromium.org/294123009/diff/20001/sdk/lib/_internal/pub/lib/src/barback/excluding_transformer.dart#newcode27 ...

6 years, 7 months ago (2014-05-27 17:40:19 UTC) #4

nweiz

6 years, 6 months ago (2014-05-27 20:36:49 UTC) #5

Message was sent while issue was closed.

Follow-up CL: https://codereview.chromium.org/307543004

https://codereview.chromium.org/294123009/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/excluding_transformer.dart (right):

https://codereview.chromium.org/294123009/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/excluding_transformer.dart:27: inner as
DeclaringTransformer, includes, excludes);
On 2014/05/27 17:40:19, Bob Nystrom wrote:
> Based on my reading of the spec, these "as" expressions should not be
necessary.
> Can you file a bug for them and add a TODO?

Done.

https://codereview.chromium.org/294123009/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/excluding_transformer.dart:72: :
super._(inner as Transformer, includes, excludes);
On 2014/05/27 17:40:19, Bob Nystrom wrote:
> I know we discussed this, but I can't recall the details. Does this mean
> DeclaringTransformer does not implement/extend Transformer? Why not?

I don't remember our original reason for not making DeclaringTransformer
implement Transformer. Maybe it was to make it clear what method it added? I
think right now I'd make the other choice, but that would technically be
backwards incompatible :-/.

Expand Messages | Collapse Messages