dart2js: Use correct call structures throughout the backend.

dart2js: Use correct call structures throughout the backend.

When translating to CPS, call arguments are normalized.  For known
targets, this means default values are passed for missing optional
arguments and named arguments are in a canonical order.  For unknown
targets, this means that passed named arguments are in a canonical
order.

However, the CallStructure recorded is one that describes the original
call.  This may have an incorrect number of passed argument or an
incorrect order for named arguments.  With this change, a new call
structure is created describing the call as it appears in the CPS IR.

R=johnniwinther@google.com

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

[Kevin Millikin (Google), 2015-12-02 15:45:30]

kmillikin@google.com changed reviewers:
+ asgerf@google.com, johnniwinther@google.com

[Kevin Millikin (Google), 2015-12-02 15:45:31]

I got tired of missing arguments and wrong argument order in the S-expression
printed representation.  What do you think of making selectors and call
structures in IR instructions reflect the actual call?

Surprisingly to me, there doesn't seem to breaks with this change.

I didn't figure out if the no such method invocation mirror wants the original
selector or not.  If it makes a difference, we should have tests for it.

The cost is increased allocation of CallStructures and Selectors.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1345:
elements.getSelector(node),
It's not obvious whether this selector should match the normalized arguments or
not.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1360:
elements.getSelector(node),
It's not obvious whether this selector should match the normalized arguments or
not.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2087: return
buildStaticNoSuchMethod(selector, normalizedArguments);
This selector doesn't seem to matter because it looks like only the name is
used.

It's not clear if it should match the normalized arguments or not, or if we
care.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2099: return
buildStaticNoSuchMethod(elements.getSelector(node.send),
It's not clear if this selector should match the normalized arguments or not.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2107: return
buildStaticNoSuchMethod(elements.getSelector(node), []);
It's not clear if this selector should match the normalized arguments or not.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2116: return
buildStaticNoSuchMethod(elements.getSelector(node),
It's not clear if this selector should match the normalized arguments or not.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2132: return
buildStaticNoSuchMethod(
It's not clear if this selector should match the normalized arguments or not. 
Here it does.  At the other call sites for buildStaticNoSuchMethod it does not.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2142: return
buildStaticNoSuchMethod(elements.getSelector(node), [visit(rhs)]);
It's not clear if this selector should match the normalized arguments or not.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2359: new
Selector.call(setter.memberName, normalizedCallStructure),
It's not obvious if this selector should match the normalized arguments or not. 
Here it does (at the other two call sites for buildInstanceNoSuchMethod it does
not).

Apparently, we don't have tests that care.

[Johnni Winther, 2015-12-03 12:15:42]

lgtm

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1345:
elements.getSelector(node),
On 2015/12/02 15:45:31, Kevin Millikin (Google) wrote:
> It's not obvious whether this selector should match the normalized arguments
or
> not.

We should. Create the new selector by:

CallStructure callStructure = translateDyn...
Selector selector = new Selector.call(method.memberName, callStructure);

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1360:
elements.getSelector(node),
On 2015/12/02 15:45:31, Kevin Millikin (Google) wrote:
> It's not obvious whether this selector should match the normalized arguments
or
> not.

Also here. Obtain the member name from `elements.getSelector(node).memberName`
and a TODO for to supply a [Name] in the visitor.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2087: return
buildStaticNoSuchMethod(selector, normalizedArguments);
On 2015/12/02 15:45:31, Kevin Millikin (Google) wrote:
> This selector doesn't seem to matter because it looks like only the name is
> used.
> 
> It's not clear if it should match the normalized arguments or not, or if we
> care.

Similar to the super-cases above. We should normalize but we need a [Name].
Generally [buildStaticNoSuchMethod] shouldn't really use [Selector].

[Kevin Millikin (Google), 2015-12-09 11:57:15]

Description was changed from

==========
dart2js: Use correct call structures throughout the backend.

When translating to CPS, call arguments are normalized.  For known
targets, this means default values are passed for missing optional
arguments and named arguments are in a canonical order.  For unknown
targets, this means that passed named arguments are in a canonical
order.

However, the CallStructure recorded is one that describes the original
call.  This may have an incorrect number of passed argument or an
incorrect order for named arguments.  With this change, a new call
structure is created describing the call as it appears in the CPS IR.
==========

to

==========
dart2js: Use correct call structures throughout the backend.

When translating to CPS, call arguments are normalized.  For known
targets, this means default values are passed for missing optional
arguments and named arguments are in a canonical order.  For unknown
targets, this means that passed named arguments are in a canonical
order.

However, the CallStructure recorded is one that describes the original
call.  This may have an incorrect number of passed argument or an
incorrect order for named arguments.  With this change, a new call
structure is created describing the call as it appears in the CPS IR.

R=johnniwinther@google.com

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

[Kevin Millikin (Google), 2015-12-09 11:57:15]

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

[Kevin Millikin (Google), 2015-12-09 12:49:13]

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1345:
elements.getSelector(node),
On 2015/12/03 12:15:42, Johnni Winther wrote:
> On 2015/12/02 15:45:31, Kevin Millikin (Google) wrote:
> > It's not obvious whether this selector should match the normalized arguments
> or
> > not.
> 
> We should. Create the new selector by:
> 
> CallStructure callStructure = translateDyn...
> Selector selector = new Selector.call(method.memberName, callStructure);

Done.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1360:
elements.getSelector(node),
On 2015/12/03 12:15:42, Johnni Winther wrote:
> On 2015/12/02 15:45:31, Kevin Millikin (Google) wrote:
> > It's not obvious whether this selector should match the normalized arguments
> or
> > not.
> 
> Also here. Obtain the member name from `elements.getSelector(node).memberName`
> and a TODO for to supply a [Name] in the visitor.

Done.

https://codereview.chromium.org/1491973002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:2087: return
buildStaticNoSuchMethod(selector, normalizedArguments);
On 2015/12/03 12:15:42, Johnni Winther wrote:
> On 2015/12/02 15:45:31, Kevin Millikin (Google) wrote:
> > This selector doesn't seem to matter because it looks like only the name is
> > used.
> > 
> > It's not clear if it should match the normalized arguments or not, or if we
> > care.
> 
> Similar to the super-cases above. We should normalize but we need a [Name].
> Generally [buildStaticNoSuchMethod] shouldn't really use [Selector].

Done.