Migrate DDC to new Analyzer task model

[vsmenon]

I have a CL out for this: https://codereview.chromium.org/1355893003/

It's blocked by the following regressions / issues I've hit so far:

• ordering and type inference across variables, see task model: inference steps for instance variables are not well-ordered #354
• Object members (hashCode, toString) result in dynamic operations (dload, dsend)
• Field type inference: initializer type is overriding lhs type - we shouldn't infer in these cases, see task mode: field initializer inference is overriding explicit LHS type #350
• Infer 'bottom' for JS('-dynamic', ...).
• Type promotion regression, see task model: type promotion regression in some cases #351
• Triage checker_test regressions (3/81 tests failing)
• Triage inference test regressions (5/60 tests failing)
• Not recognizing / trigger some dinvokes (see below)
• Parameter inference not working on setter overrides (see below)
• Inference and generic substitution error (see below)
• Triage codegen test failures / stack traces, see task model: fails to correctly restore resolved AST after it is flushed from cache  #352
• initializing formals, see task model: not correctly checking (inferring?) initializing formals #346
• transitive instance/field inference not working, see task model: transitive instance/field inference not working  #347
• infer final fields where subtype narrows type, see task model: override inference not working for final fields when subtype narrows the type #348
• should not infer if explicitly marked dynamic, see task model: inferring type of variable that is explicit dynamic #349
• Classes with call methods should subtype function type, see task model: Classes with call methods don't type check #356

[vsmenon]

For const fields, a declaration in Dart:

const GLOBAL_FUNCTIONS = 'globalFunctions';

is producing the following JS with a cast to String at a use site:

_foreign_helper.JS_EMBEDDED_GLOBAL("", dart.as(_js_embedded_names.GLOBAL_FUNCTIONS, core.String));

[vsmenon]

For the field type inference issue, we're seeing extra casts like:

Zone._current = dart.as(zone, _RootZone);

even though _current is declared in Dart as:

  static Zone _current = _ROOT_ZONE;

It appears that the rhs expression type is incorrectly overriding the lhs declared type.

[vsmenon]

I'm seeing the following type promo regression:

Original Dart code:

    if (index is num && index == index.toInt()) {
      _checkIndex(index);
    }

Old generated JS:

   if (typeof index == 'number' && index == index[dartx.toInt]()) {
     this[_checkIndex](index);
   }

New (with task model CL) generated JS:

   if (typeof index == 'number' && dart.equals(index, dart.dsend(index, 'toInt'))) {
     this[_checkIndex](dart.as(index, core.int));
   }

We seem to have lost the type promotion on index.

[vsmenon]

This code is now neither triggering an error/warning/info nor generating a dinvoke on the call to toString():

main() {
  dynamic toString = () => null;
  toString = (x) => x;
  toString();
}

[vsmenon]

Parameter inference appears to broken for setter override. We used to infer the setter type for Child.x, but with the task model version, we're getting an override error.

abstract class Base {
  void set x(int value);
}

class Child extends Base {
  void set x(value) {}
}

[jmesserly]

I think that's in strong_mode.dart, _inferAccessor ... I see it setting the field type and getter return type but not the setter parameter type. Also FWIW, if there's no getter, it doesn't look like it will hit the setter path at all

[vsmenon]

I'm seeing the following problem with inference that appears to be related to generic type param substitution. I'm now getting an analyzer message "A value of type 'E' cannot be assigned to a variable of type 'String' (foo.dart, line 14, col 14)" in main below. E is not in scope here. It appears that B.self is inferred to type A.

class A<E> {
  E value;

  A<E> get self => this;
}

class B<E> extends A<E> {
  B(E v) { this.value = v; }

  get self => this;
}

void main() {
  String v = new B<String>('hello').self.value;
  print(v);
}

[jmesserly]

I'm seeing the following problem with inference that appears to be related to generic type param substitution.

edit: guess here was wrong. see #342 (comment)

yup, that's in almost the same code I'm looking at (edit: which is _inferMethod in analyzer/lib/src/task/strong_mode.dart). You can see the bug more clearly if you give those two generic parameters different names. Say we make A<E> be A<X> instead. It will infer B.self to be A<X> ... not A<E> which would be the correct substitution.

What our old code did (_inferMethodTypesFromOverride) is to go from the Type, walk into mixins/supertypes/interfaces, and then get the FunctionType of the method (or getter, setter). Because we're walking the Type side of things, substitution of generics is handled. So the supertype of B<E> will be A<E> not A<X>

[jmesserly]

... and it looks like we can probably do something along the lines of substituteTypeArgumentsInMemberFromInheritance (called by lookupMemberType) in InheritanceManager.

[jmesserly]

here's the setter fix: https://codereview.chromium.org/1359243003 ... working on generics

[jmesserly]

Hmmm, I'm not able to reproduce the generics issue with InstanceMemberInferrer. Going to investigate further...

EDIT: further investigation, it seems InheritanceManager.lookupOverrides does the correct substitution (it returns a PropertyAccessorMember which has correct return type A<E> instead of A<X> in my example). So wherever the bug lies, it's not in InstanceMemberInferrer. Perhaps there's an ordering issue of some sort, or some other part of the system is responsible.

[jmesserly]

Okay progress on the generic issue: the test is a little more tricky, it doesn't look like a problem with inferred return types. It looks like the methodinvocation node's methodname is not pointing at the right staticElement and/or has the incorrect staticType. So it might be related to the other issue where we get the function type that corresponds to the method's SimpleIdentifier tear-off.

edit: I'm going to see what the data structures looked like when our old implementation reached this point in the code.

edit2: confirmed, it's the MethodInvocation.methodName.staticElement. In the old code/analyzer, it was correctly substituted generic MethodMember, in the new code/analyzer, it's not, instead it's the raw MethodElementImpl with an E return type. Now to figure out where that's coming from...

[jmesserly]

Think I have the generics issue figured out. If we're going to infer a return or parameter type that has an unsubstituted generic arg, we need to make sure the FunctionType also has a generic parameter. Otherwise substitution will fail. Tests & fix here: https://codereview.chromium.org/1364353003

edit: CL is landed, so should be fixed now

[jmesserly]

@vsmenon I believe all issues are now triaged in checker_test and inference_test. I created 4 different bugs, I'll create bugs as well for the 3 issues that don't have bugs yet.

[jmesserly]

@vsmenon -- do you know if this still repros: "Const type inference doesn't work - at least in some cases."

[jmesserly]

Also if you recall, what was the context for "Not recognizing / trigger some dinvokes (see below)"

[jmesserly]

Filed bugs 346-352 for following up with the various issues. Let me know if you remember what these two were, and we can split off bugs for them as well:

Const type inference doesn't work - at least in some cases.
Not recognizing / trigger some dinvokes (see below)

[jmesserly]

Whew! #352 #354 are left, the other fixes are in. Unless we find anything else it's looking pretty good...

[jmesserly]

CL out for #356

[vsmenon]

This is landed.

[jmesserly]

🎆

[jmesserly]

AWESOME!