Move computation of method flags into model.

Move computation of method flags into model.

BUG=
R=floitsch@google.com, zarah@google.com

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

[herhut, 2015-01-28 13:41:49]

First step is creating Methods and using them in the old emitter. 

Next: Move stubs into model.

[floitsch, 2015-01-28 16:10:39]

LGTM.

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

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/model.dart:300: class DartMethod extends Method
{
Add comment what a "DartMethod" is.

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/model.dart:307: final bool canBeReflected;
It would be an option to keep the "canBeReflected" part out of here.
The whole reflection code is only in the old emitter anyway.

I'm fine either way.

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/model.dart:311: this.canBeReflected})
nit: indent one less.

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/model.dart:335: needsTearOff: needsTearOff,
I would align with "(".

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/model.dart:357: needsTearOff: needsTearOff,
I would align with the "(".

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/model.dart:369: assert(needsTearOff == false);
Will this change?
If not, why get it as an argument?

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
File pkg/compiler/lib/src/js_emitter/old_emitter/class_emitter.dart (right):

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/old_emitter/class_emitter.dart:239: //
TODO(herhut): This is a no-op. Should it be removed?
In a different CL.

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right):

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:746: // We need to
filter out null-elements for the interceptors.
That works too :)

Please add a:
TODO(floitsch): use the precomputed interceptors here.

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

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/program_builder.dart:395: Method
buildMethodForTry(FunctionElement element) {
As discussed: change name.

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/program_builder.dart:405: // TODO(herhut): Why
does this get code passed in and statics don't?
Probably just an oversight.

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js_emitter/program_builder.dart:484: // TODO(herhut): Can
these actually ever be torn off?
Until we find a case where it can be, let's just remove it and assume that "no".

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/dart2js_increment...
File pkg/dart2js_incremental/lib/library_updater.dart (right):

https://chromiumcodereview.appspot.com/882713008/diff/1/pkg/dart2js_increment...
pkg/dart2js_incremental/lib/library_updater.dart:1469: ProgramBuilder get
programBuilder {
I wouldn't do this. The calls to the programBuilder are relatively hackish and I
prefer keeping the full code together.

[herhut, 2015-01-29 10:24:21]

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

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/model.dart:300: class DartMethod extends Method
{
On 2015/01/28 16:10:39, floitsch wrote:
> Add comment what a "DartMethod" is.

Done.

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/model.dart:307: final bool canBeReflected;
On 2015/01/28 16:10:39, floitsch wrote:
> It would be an option to keep the "canBeReflected" part out of here.
> The whole reflection code is only in the old emitter anyway.
> 
> I'm fine either way.

I will remove it again later if it can be pushed into the old emitter easily.

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/model.dart:311: this.canBeReflected})
On 2015/01/28 16:10:39, floitsch wrote:
> nit: indent one less.

Done.

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/model.dart:335: needsTearOff: needsTearOff,
On 2015/01/28 16:10:39, floitsch wrote:
> I would align with "(".

Done.

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/model.dart:357: needsTearOff: needsTearOff,
On 2015/01/28 16:10:39, floitsch wrote:
> I would align with the "(".

Done.

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/model.dart:369: assert(needsTearOff == false);
On 2015/01/28 16:10:39, floitsch wrote:
> Will this change?
> If not, why get it as an argument?

The original callsites to this had the tearoff parameter with a TODO to compute
its value. Currently, there is no place to put the tearoff in the model, but I
did not want to remove the TODO until I am sure it is always false.

Removed now.

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

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/old_emitter/class_emitter.dart:239: //
TODO(herhut): This is a no-op. Should it be removed?
On 2015/01/28 16:10:39, floitsch wrote:
> In a different CL.

Acknowledged.

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

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:746: // We need to
filter out null-elements for the interceptors.
On 2015/01/28 16:10:39, floitsch wrote:
> That works too :)
> 
> Please add a:
> TODO(floitsch): use the precomputed interceptors here.

Done.

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

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/program_builder.dart:395: Method
buildMethodForTry(FunctionElement element) {
On 2015/01/28 16:10:39, floitsch wrote:
> As discussed: change name.

now buildMethodHackForIncrementalCompilation

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/program_builder.dart:405: // TODO(herhut): Why
does this get code passed in and statics don't?
On 2015/01/28 16:10:39, floitsch wrote:
> Probably just an oversight.

Changed.

https://codereview.chromium.org/882713008/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/program_builder.dart:484: // TODO(herhut): Can
these actually ever be torn off?
On 2015/01/28 16:10:39, floitsch wrote:
> Until we find a case where it can be, let's just remove it and assume that
"no".

Done.

https://codereview.chromium.org/882713008/diff/1/pkg/dart2js_incremental/lib/...
File pkg/dart2js_incremental/lib/library_updater.dart (right):

https://codereview.chromium.org/882713008/diff/1/pkg/dart2js_incremental/lib/...
pkg/dart2js_incremental/lib/library_updater.dart:1469: ProgramBuilder get
programBuilder {
On 2015/01/28 16:10:39, floitsch wrote:
> I wouldn't do this. The calls to the programBuilder are relatively hackish and
I
> prefer keeping the full code together.

Done.

[herhut, 2015-01-29 12:56:18]

Committed patchset #3 (id:40001) manually as 43279 (presubmit successful).