Build CFG for various JS() foreign methods

Build CFG for various JS() foreign methods

BUG=
R=het@google.com

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

Reverted:  https://github.com/dart-lang/sdk/commit/476ffbda5e24973ce39ce8fae57d144054755ca8
Committed: https://github.com/dart-lang/sdk/commit/ed4c2cb457080715f5f564abd4474236a71a73da

[sra1, 2016-10-12 03:39:10]

Patchset #1 (id:1) has been deleted

[sra1, 2016-10-12 03:40:03]

Description was changed from

==========
Compile JS()

BUG=
==========

to

==========
Build CFG for various JS() foreign methods

BUG=
==========

[sra1, 2016-10-12 03:40:03]

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

[sra1, 2016-10-12 03:40:18]

Patchset #1 (id:20001) has been deleted

[sra1, 2016-10-12 03:43:04]

I'm not finished with all the JS_xxx functions, but I thought you might like to
get started on the review.

[Harry Terkelsen, 2016-10-12 16:42:59]

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_visitor.dart (right):

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_visitor.dart:2140: return
associateNode(buildStaticInvoke(function, arguments, isConst: false), node);
dartfmt

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:746: return '$count ${plural}s';
should be '$count $plural'

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:801: dynamic
_foreignConstantEnumArgument(
this seems unused?

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:811: 'text': "Error: Expected
String constant as ${adjective}argument "
should it be 'Expected enum constant ...'?

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:818: StringConstantValue
stringConstant = hConstant.constant;
should this be an EnumConstantValue?

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:995: assert(nativeBehavior !=
null);
remove this assert since it makes the next one unreachable

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:1023: if
(nativeBehavior.codeTemplate.isExpression) {
maybe rewrite this entire if statement as:

push(new HForeignCode(nativeBehavior.codeTemplate, ssaType, inputs,
           isStatement: !nativeBehavior.codeTemplate.isExpression,
           effects: nativeBehavior.sideEffects,
           nativeBehavior: nativeBehavior)
         ..sourceInformation = sourceInformation);

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

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:282: //bool
isInForeignLibrary(ir.Member member) =>
remove this comment?

[sra1, 2016-10-13 19:02:01]

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_visitor.dart (right):

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_visitor.dart:2140: return
associateNode(buildStaticInvoke(function, arguments, isConst: false), node);
On 2016/10/12 16:42:58, Harry Terkelsen wrote:
> dartfmt

Done.

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:746: return '$count ${plural}s';
On 2016/10/12 16:42:59, Harry Terkelsen wrote:
> should be '$count $plural'

Done.

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:801: dynamic
_foreignConstantEnumArgument(
On 2016/10/12 16:42:59, Harry Terkelsen wrote:
> this seems unused?

Removed

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:811: 'text': "Error: Expected
String constant as ${adjective}argument "
On 2016/10/12 16:42:59, Harry Terkelsen wrote:
> should it be 'Expected enum constant ...'?

Acknowledged.

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:818: StringConstantValue
stringConstant = hConstant.constant;
On 2016/10/12 16:42:59, Harry Terkelsen wrote:
> should this be an EnumConstantValue?

Acknowledged.

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:995: assert(nativeBehavior !=
null);
On 2016/10/12 16:42:59, Harry Terkelsen wrote:
> remove this assert since it makes the next one unreachable

Done.

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:1023: if
(nativeBehavior.codeTemplate.isExpression) {
On 2016/10/12 16:42:58, Harry Terkelsen wrote:
> maybe rewrite this entire if statement as:
> 
> push(new HForeignCode(nativeBehavior.codeTemplate, ssaType, inputs,
>            isStatement: !nativeBehavior.codeTemplate.isExpression,
>            effects: nativeBehavior.sideEffects,
>            nativeBehavior: nativeBehavior)
>          ..sourceInformation = sourceInformation);

Done.

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

https://codereview.chromium.org/2404123002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:282: //bool
isInForeignLibrary(ir.Member member) =>
On 2016/10/12 16:42:59, Harry Terkelsen wrote:
> remove this comment?

Done.

[Harry Terkelsen, 2016-10-13 20:36:51]

lgtm

[sra1, 2016-10-14 00:11:54]

Description was changed from

==========
Build CFG for various JS() foreign methods

BUG=
==========

to

==========
Build CFG for various JS() foreign methods

BUG=
R=het@google.com

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

[sra1, 2016-10-14 00:11:55]

Committed patchset #3 (id:80001) manually as
a897bc383247592379ce5148652b78678a4d0f97 (presubmit successful).

[sra1, 2016-10-14 02:08:42]

Description was changed from

==========
Build CFG for various JS() foreign methods

BUG=
R=het@google.com

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

to

==========
Build CFG for various JS() foreign methods

BUG=
R=het@google.com

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

Reverted: 
https://github.com/dart-lang/sdk/commit/476ffbda5e24973ce39ce8fae57d144054755ca8
==========

[sra1, 2016-11-21 22:24:43]

Description was changed from

==========
Build CFG for various JS() foreign methods

BUG=
R=het@google.com

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

Reverted: 
https://github.com/dart-lang/sdk/commit/476ffbda5e24973ce39ce8fae57d144054755ca8
==========

to

==========
Build CFG for various JS() foreign methods

BUG=
R=het@google.com

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

Reverted: 
https://github.com/dart-lang/sdk/commit/476ffbda5e24973ce39ce8fae57d144054755ca8
Committed:
https://github.com/dart-lang/sdk/commit/ed4c2cb457080715f5f564abd4474236a71a73da
==========

[sra1, 2016-11-21 22:24:43]

Committed patchset #9 (id:200001) manually as
ed4c2cb457080715f5f564abd4474236a71a73da (presubmit successful).