dart2js: Accept named holes in js-templates.

dart2js: Accept named holes in js-templates.

R=sigurdm@google.com, sra@google.com

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

[floitsch, 2014-10-22 11:05:35]

floitsch@google.com changed reviewers:
+ sra@google.com

[sra1, 2014-10-22 21:50:29]

lgtm

https://chromiumcodereview.appspot.com/671513013/diff/1/sdk/lib/_internal/com...
File sdk/lib/_internal/compiler/implementation/js/template.dart (right):

https://chromiumcodereview.appspot.com/671513013/diff/1/sdk/lib/_internal/com...
sdk/lib/_internal/compiler/implementation/js/template.dart:105: assert(arguments
is Map);
It might be nice to collect the set of names in the template and verify that the
map provides exactly those names.

[sra1, 2014-10-22 21:56:19]

There is a test for this - dart2js/js_parser_statements_test

I would add #A and #{A} versions of most of the tests.
I don't mind if you insist on only one of these named forms.

Please also test error conditions.

[floitsch, 2014-11-03 18:12:59]

PTAL.

https://codereview.chromium.org/671513013/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js/template.dart (right):

https://codereview.chromium.org/671513013/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js/template.dart:105: assert(arguments
is Map);
On 2014/10/22 21:50:28, sra1 wrote:
> It might be nice to collect the set of names in the template and verify that
the
> map provides exactly those names.

Done.

[floitsch, 2014-11-06 13:14:49]

ping. PTAL.

[sra1, 2014-11-06 17:47:14]

lgtm

[floitsch, 2014-11-07 10:27:13]

floitsch@google.com changed reviewers:
+ sigurdm@google.com

[floitsch, 2014-11-07 10:27:13]

@Sigurd: could you have a look at the changes in the emitter?
I moved the finishedClasses to the buildFinishClass, and I renamed
finishClassCall to finishClassFunction.

[sigurdm, 2014-11-07 10:42:35]

Changes lgtm
I am not sure what you meant by the finishClassCall/finishClassFunction rename.
I don't see a rename in the patch

[floitsch, 2014-11-07 12:45:14]

On 2014/11/07 10:42:35, sigurdm wrote:
> Changes lgtm
> I am not sure what you meant by the finishClassCall/finishClassFunction
rename.
> I don't see a rename in the patch

Sorry. That was confusing. The rename is compared to an earlier patch set.

[floitsch, 2014-11-07 12:45:57]

Committed patchset #4 (id:60001) manually as 41601 (presubmit successful).