Refactor computation of NativeBehavior.

Refactor computation of NativeBehavior.

- to prepare for kernel based implementation.

R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/a2837b78d51aa79e85822d701555f538084b08b6

[Johnni Winther, 2017-02-08 11:07:29]

Description was changed from

==========
Refactor computation of NativeBehavior.
==========

to

==========
Refactor computation of NativeBehavior.

- to prepare for kernel based implementation.
==========

[Johnni Winther, 2017-02-08 11:07:38]

johnniwinther@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2017-02-08 17:00:28]

lgtm

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/na...
File pkg/compiler/lib/src/native/behavior.dart (right):

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/behavior.dart:830: abstract class BehaviorComputer {
nit: rename as BehaviorBuilder or BehaviorFactory

(I prefer builder)

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/behavior.dart:963: NativeBehavior
ofFieldLoad(DartType type,
Let's rename these now that they are no longer on NativeBehavior (the
"ofFieldLoad" made sense when read in context like
"NativeBehavior.ofFieldLoad").

Suggestions:
 - buildFieldLoadBehavior
 - fieldLoadBehaviorOf

I prefer the former (builderFieldLoadBehavior) since it's not expilcit in the
arguments what is the field (what the "Of" is about).

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/behavior.dart:1021: class BehaviorComputerImpl
extends BehaviorComputer {
nit: rename to ResolverBehaviorBuilder? (so the new one can be called
KernelBehaviorBuilder?)

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart (right):

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:599: // TODO(johnniwinther):
Provide the correct value for [isJsInterop].
was this working before? (are there tests that will stop working after this
change as a result?) - that is OK, but it would be good to know.

[Johnni Winther, 2017-02-09 10:20:43]

Description was changed from

==========
Refactor computation of NativeBehavior.

- to prepare for kernel based implementation.
==========

to

==========
Refactor computation of NativeBehavior.

- to prepare for kernel based implementation.

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/a2837b78d51aa79e85822d701555f538084b08b6
==========

[Johnni Winther, 2017-02-09 10:20:44]

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

[Johnni Winther, 2017-02-10 09:11:46]

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/na...
File pkg/compiler/lib/src/native/behavior.dart (right):

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/behavior.dart:830: abstract class BehaviorComputer {
On 2017/02/08 17:00:28, Siggi Cherem (dart-lang) wrote:
> nit: rename as BehaviorBuilder or BehaviorFactory
> 
> (I prefer builder)

Done.

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/behavior.dart:963: NativeBehavior
ofFieldLoad(DartType type,
On 2017/02/08 17:00:28, Siggi Cherem (dart-lang) wrote:
> Let's rename these now that they are no longer on NativeBehavior (the
> "ofFieldLoad" made sense when read in context like
> "NativeBehavior.ofFieldLoad").
> 
> Suggestions:
>  - buildFieldLoadBehavior
>  - fieldLoadBehaviorOf
> 
> I prefer the former (builderFieldLoadBehavior) since it's not expilcit in the
> arguments what is the field (what the "Of" is about). 

Done.

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/behavior.dart:1021: class BehaviorComputerImpl
extends BehaviorComputer {
On 2017/02/08 17:00:28, Siggi Cherem (dart-lang) wrote:
> nit: rename to ResolverBehaviorBuilder? (so the new one can be called
> KernelBehaviorBuilder?)

Done.

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart (right):

https://codereview.chromium.org/2686533002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:599: // TODO(johnniwinther):
Provide the correct value for [isJsInterop].
On 2017/02/08 17:00:28, Siggi Cherem (dart-lang) wrote:
> was this working before? (are there tests that will stop working after this
> change as a result?) - that is OK, but it would be good to know.

Same behavior as before (probably not working).