Remove use of FunctionSignature et al from call_structure.dart

Remove use of FunctionSignature et al from call_structure.dart

R=sigmund@google.com

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

[Johnni Winther, 2017-01-13 13:38:34]

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

[Siggi Cherem (dart-lang), 2017-01-13 17:11:07]

small suggestions, otherwise lgtm

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/compil...
File pkg/compiler/lib/src/compile_time_constants.dart (right):

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:1300:
target.functionSignature.parameterCount, target.type.namedParameters);
do we want to also get rid of functionSignature here?
 (compute parameterCount from target.type instead)?

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/elemen...
File pkg/compiler/lib/src/elements/elements.dart (right):

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/elemen...
pkg/compiler/lib/src/elements/elements.dart:830: * Precondition:
`this.applies(element, world)`.
what is `this` in this static context?

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/univer...
File pkg/compiler/lib/src/universe/call_structure.dart (right):

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/univer...
pkg/compiler/lib/src/universe/call_structure.dart:103: if (name ==
namedParameters[nameIndex]) {
Can we get here anymore with the duplicated names, or will the resolver prevent
us from reaching this point? If not, we could move the ++ here to ensure the
same semantics:
  if (name == namedParameters[nameIndex++])

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/univer...
pkg/compiler/lib/src/universe/call_structure.dart:104: continue OUTER;
I find the logic with `continue` a bit hard to follow. This has an extra check,
but might be more self explanatory:

for (String name in getOrderedNamedArguments()) {
  bool found = false;
  // Note: we start at the existing index because arguments are sorted.
  while (nameIndex < namedParameters.length) {
    if (name == namedParameters[nameIndex]) {
      found = true;
      break;
    }
    nameIndex++; 
  }
  if (!found) return false;
}
return true;

[Johnni Winther, 2017-01-18 11:02:05]

Description was changed from

==========
Remove use of FunctionSignature et al from call_structure.dart
==========

to

==========
Remove use of FunctionSignature et al from call_structure.dart

R=sigmund@google.com

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

[Johnni Winther, 2017-01-18 11:02:06]

Committed patchset #1 (id:1) manually as
7713e62409d06668b40bfd027f80e70ce6394b83 (presubmit successful).

[Johnni Winther, 2017-01-18 11:02:51]

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/compil...
File pkg/compiler/lib/src/compile_time_constants.dart (right):

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:1300:
target.functionSignature.parameterCount, target.type.namedParameters);
On 2017/01/13 17:11:07, Siggi Cherem (dart-lang) wrote:
> do we want to also get rid of functionSignature here?
>  (compute parameterCount from target.type instead)?

I considered adding a 'parameterCount' property to FunctionType, but we
currently have a lot of inconsistent semantics on the various
(-/required/positional/named)ParameterCount properties so for this CL I didn't
want to add to the madness. Will look into it later.

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/elemen...
File pkg/compiler/lib/src/elements/elements.dart (right):

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/elemen...
pkg/compiler/lib/src/elements/elements.dart:830: * Precondition:
`this.applies(element, world)`.
On 2017/01/13 17:11:07, Siggi Cherem (dart-lang) wrote:
> what is `this` in this static context?

Old invariant. Changed to `callStructure.signatureApplies(element.type)`

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/univer...
File pkg/compiler/lib/src/universe/call_structure.dart (right):

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/univer...
pkg/compiler/lib/src/universe/call_structure.dart:103: if (name ==
namedParameters[nameIndex]) {
On 2017/01/13 17:11:07, Siggi Cherem (dart-lang) wrote:
> Can we get here anymore with the duplicated names, or will the resolver
prevent
> us from reaching this point? If not, we could move the ++ here to ensure the
> same semantics:
>   if (name == namedParameters[nameIndex++])

We still create both function signatures/types and call structures with
duplicate names, but these are erroneous cases so what action we take here is
not that important. It's even preferable to allow calls with duplicate names to
methods with duplicate names...

https://codereview.chromium.org/2630743002/diff/1/pkg/compiler/lib/src/univer...
pkg/compiler/lib/src/universe/call_structure.dart:104: continue OUTER;
On 2017/01/13 17:11:07, Siggi Cherem (dart-lang) wrote:
> I find the logic with `continue` a bit hard to follow. This has an extra
check,
> but might be more self explanatory:
> 
> for (String name in getOrderedNamedArguments()) {
>   bool found = false;
>   // Note: we start at the existing index because arguments are sorted.
>   while (nameIndex < namedParameters.length) {
>     if (name == namedParameters[nameIndex]) {
>       found = true;
>       break;
>     }
>     nameIndex++; 
>   }
>   if (!found) return false;
> }
> return true;
>     

Done.