Remove some duplicate code in FunctionTypeImpl.

Remove some duplicate code in FunctionTypeImpl.

I noticed it when I was in there for LUB, so I figured I'd do some
spring cleaning. Changes are:

- Consolidate duplicate code in namedParameterTypes,
  normalParameterTypes, and optionalParameterTypes into a shared
  _forEachParameterType() method.
- Consolidate duplicate code in isSubtypeOf() and isMoreSpecificThan()
  into a shared _testTypeRelation() method that is parameterized over
  the specific relation.
- Also some other small-scale changes in the relation code.

R=paulberry@google.com

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

[Bob Nystrom, 2016-02-03 01:43:17]

rnystrom@google.com changed reviewers:
+ paulberry@google.com

[Paul Berry, 2016-02-03 02:13:53]

Nice cleanup.  lgtm assuming comments are addressed.

https://codereview.chromium.org/1662873002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/type.dart (right):

https://codereview.chromium.org/1662873002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/type.dart:691: type, (t, s) =>
t.isMoreSpecificThan(s, withDynamic));
In analyzer code, we fully type all variables.  So this should be:

    (TypeImpl t, TypeImpl s) => t.isMoreSpecificThan(...)

https://codereview.chromium.org/1662873002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/type.dart:696: return _testTypeRelation(type,
(t, s) => t.isAssignableTo(s));
Similar issue here.

https://codereview.chromium.org/1662873002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/type.dart:745: if (!tNamed.containsKey(key)) {
Nit: we're doing redundant map lookups here.  It's more efficient to do:

    DartType namedParamType = tNamed[key];
    if (namedParamType == null || !relation(namedParamType, sNamed[key]) {
      return false;
    }

Which is what the code used to do before the refactor.

(Normally I wouldn't worry about this, on the grounds that we should let
profiling results guide us to where optimization is necessary.  But in this
case, since you're refactoring code that *had* this optimization, and I believe
it's a fairly hot code path, I think we should preserve the optimization.)

[Bob Nystrom, 2016-02-03 20:49:49]

https://codereview.chromium.org/1662873002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/type.dart (right):

https://codereview.chromium.org/1662873002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/type.dart:691: type, (t, s) =>
t.isMoreSpecificThan(s, withDynamic));
On 2016/02/03 02:13:53, Paul Berry wrote:
> In analyzer code, we fully type all variables.  So this should be:
> 
>     (TypeImpl t, TypeImpl s) => t.isMoreSpecificThan(...)

Oops. Still learning all the analyzer style rules. Done!

https://codereview.chromium.org/1662873002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/type.dart:696: return _testTypeRelation(type,
(t, s) => t.isAssignableTo(s));
On 2016/02/03 02:13:53, Paul Berry wrote:
> Similar issue here.

Done.

https://codereview.chromium.org/1662873002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/type.dart:745: if (!tNamed.containsKey(key)) {
On 2016/02/03 02:13:53, Paul Berry wrote:
> Nit: we're doing redundant map lookups here.  It's more efficient to do:
> 
>     DartType namedParamType = tNamed[key];
>     if (namedParamType == null || !relation(namedParamType, sNamed[key]) {
>       return false;
>     }
> 
> Which is what the code used to do before the refactor.
> 
> (Normally I wouldn't worry about this, on the grounds that we should let
> profiling results guide us to where optimization is necessary.  But in this
> case, since you're refactoring code that *had* this optimization, and I
believe
> it's a fairly hot code path, I think we should preserve the optimization.)

Good catch. Done.

I figured this was a hot code path, so I did a few other minor optimizations
too. The old code called  .namedParameterTypes multiple times and I think had a
few other redundant calculations that I coalesced.

[Bob Nystrom, 2016-02-03 20:50:08]

Description was changed from

==========
Remove some duplicate code in FunctionTypeImpl.

I noticed it when I was in there for LUB, so I figured I'd do some
spring cleaning. Changes are:

- Consolidate duplicate code in namedParameterTypes,
  normalParameterTypes, and optionalParameterTypes into a shared
  _forEachParameterType() method.
- Consolidate duplicate code in isSubtypeOf() and isMoreSpecificThan()
  into a shared _testTypeRelation() method that is parameterized over
  the specific relation.
- Also some other small-scale changes in the relation code.
==========

to

==========
Remove some duplicate code in FunctionTypeImpl.

I noticed it when I was in there for LUB, so I figured I'd do some
spring cleaning. Changes are:

- Consolidate duplicate code in namedParameterTypes,
  normalParameterTypes, and optionalParameterTypes into a shared
  _forEachParameterType() method.
- Consolidate duplicate code in isSubtypeOf() and isMoreSpecificThan()
  into a shared _testTypeRelation() method that is parameterized over
  the specific relation.
- Also some other small-scale changes in the relation code.

R=paulberry@google.com

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

[Bob Nystrom, 2016-02-03 20:50:09]

Committed patchset #2 (id:20001) manually as
67b725986b83e37a7faa50f02c2a19b76b68d8a6 (presubmit successful).