dart2js: Move parameterStub generation to parameter_stub_generator and add parameter stubs to model.

dart2js: Move parameterStub generation to parameter_stub_generator and add parameter stubs to model.

R=herhut@google.com

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

[zarah, 2015-01-30 09:44:58]

https://codereview.chromium.org/887853004/diff/1/pkg/compiler/lib/src/js_emit...
File pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart (right):

https://codereview.chromium.org/887853004/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:30:
FunctionSignature parameters = member.functionSignature;
Copied from container_builder.

https://codereview.chromium.org/887853004/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:142: 
Change in the following line.

https://codereview.chromium.org/887853004/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:150: 
Copied from container_builder, changes only in the lines concerning
generatedStubs.

[zarah, 2015-01-30 09:53:20]

zarah@google.com changed reviewers:
+ floitsch@google.com, herhut@google.com

[herhut, 2015-01-30 10:02:22]

lgtm w/ comments

https://codereview.chromium.org/887853004/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/model.dart (right):

https://codereview.chromium.org/887853004/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/model.dart:371: parameterStubs: parameterStubs)
{
I would prefer this to be an unnamed parameter. They are mandatory and at the
call site it is clear what is being passed (other than for a boolean flag).

https://codereview.chromium.org/887853004/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/model.dart:417: parameterStubs :
parameterStubs);
As above.

https://codereview.chromium.org/887853004/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart (right):

https://codereview.chromium.org/887853004/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart:18: final
bool needsStubs = !method.parameterStubs.isEmpty;
How about isNotEmpty?

https://codereview.chromium.org/887853004/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/program_builder.dart (right):

https://codereview.chromium.org/887853004/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/program_builder.dart:463:
List<ParameterStubMethod> parameterStubs = <ParameterStubMethod>[];
How about 

if (!_methodNeedsStubs) return const <ParameterStubMethod>[];

I think it makes it more obvious that nothing happens unless a method needs
stubs.

[floitsch, 2015-01-30 12:24:45]

DBC.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:7: class
ParameterStubGenerator {
I think this class could have been merged into the class_stub_generator, but
since it's already here...

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:29: Selector
selector) {
nit: indentation.

[floitsch, 2015-01-30 12:25:24]

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:7: class
ParameterStubGenerator {
On 2015/01/30 12:24:45, floitsch wrote:
> I think this class could have been merged into the class_stub_generator, but
> since it's already here...

Please don't merge it now, since I'm going to rebase on top of this CL.

[floitsch, 2015-01-30 12:26:31]

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/model.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/model.dart:389: * A method that is generated for
the different versions of method calls of
For new code please prefer "///" comments.

[floitsch, 2015-01-30 12:33:03]

Another small one.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:1: // Copyright
(c) 2014, the Dart project authors.  Please see the AUTHORS file
2015

[floitsch, 2015-01-30 12:36:52]

another one...

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/js_emitter.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/js_emitter.dart:95: part
'parameter_stub_generator.dart';
please sort alphabetically.

[zarah, 2015-01-30 12:46:39]

Thanks.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/model.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/model.dart:371: parameterStubs: parameterStubs)
{
On 2015/01/30 10:02:22, herhut wrote:
> I would prefer this to be an unnamed parameter. They are mandatory and at the
> call site it is clear what is being passed (other than for a boolean flag).

Done.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/model.dart:389: * A method that is generated for
the different versions of method calls of
On 2015/01/30 12:26:30, floitsch wrote:
> For new code please prefer "///" comments.

Done.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/model.dart:417: parameterStubs :
parameterStubs);
On 2015/01/30 10:02:22, herhut wrote:
> As above.

Done.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart:18: final
bool needsStubs = !method.parameterStubs.isEmpty;
On 2015/01/30 10:02:22, herhut wrote:
> How about isNotEmpty?

Of course! :-)

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:1: // Copyright
(c) 2014, the Dart project authors.  Please see the AUTHORS file
On 2015/01/30 12:33:03, floitsch wrote:
> 2015

Done.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:7: class
ParameterStubGenerator {
On 2015/01/30 12:24:45, floitsch wrote:
> I think this class could have been merged into the class_stub_generator, but
> since it's already here...

I was also considering that. So I agree.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart:29: Selector
selector) {
On 2015/01/30 12:24:45, floitsch wrote:
> nit: indentation.

Done.

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js_emitter/program_builder.dart (right):

https://chromiumcodereview.appspot.com/887853004/diff/20001/pkg/compiler/lib/...
pkg/compiler/lib/src/js_emitter/program_builder.dart:463:
List<ParameterStubMethod> parameterStubs = <ParameterStubMethod>[];
On 2015/01/30 10:02:22, herhut wrote:
> How about 
> 
> if (!_methodNeedsStubs) return const <ParameterStubMethod>[];
> 
> I think it makes it more obvious that nothing happens unless a method needs
> stubs. 

Done.

[zarah, 2015-01-30 13:31:46]

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