Skip to content

Strong mode generics - how to apply correctly #25100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zoechi opened this issue Dec 3, 2015 · 5 comments
Closed

Strong mode generics - how to apply correctly #25100

zoechi opened this issue Dec 3, 2015 · 5 comments
Assignees
Labels
legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@zoechi
Copy link
Contributor

zoechi commented Dec 3, 2015

The code below produces a few errors I don't know how to fix.

UnmodifiableListView<int> _ioServiceBytesRecursive;

_ioServiceBytesRecursive = toUnmodifiableListView /*<int>*/ (json['io_service_bytes_recursive'] as Iterable);
//  WARNING: toUnmodifiableListView<int>(json['io_service_bytes_recursive'] as Iterable) (UnmodifiableListView<dynamic>) will need runtime check to cast to type UnmodifiableListView<int> ([bwu_docker] lib/src/shared/data_structures.dart:138)
...

UnmodifiableListView /*<T>*/ toUnmodifiableListView /*<T>*/ (Iterable list) {
  if (list == null) {
    return null;
  }
  if (list.length == 0) {
    return new UnmodifiableListView<dynamic /*=T*/ >(<dynamic /*=T*/ >[]);
  }

  return new UnmodifiableListView<dynamic /*=T*/ >(
      list.map /*<T>*/ ((dynamic e) {
    if (e is Map) {
      return toUnmodifiableMapView(e) as dynamic /*=T*/; 
// ERROR: Type check failed: toUnmodifiableMapView(e) as T (T) is not of type T ([bwu_docker] lib/src/shared/json_util.dart:143)
// WARNING: The return type 'T (/home/myuser/dart/bwu_docker/lib/src/shared/json_util.dart)' is not a 'T (/usr/local/apps/dart/dart-sdk/lib/core/iterable.dart)', as defined by the method '' ([bwu_docker] lib/src/shared/json_util.dart:143)

    } else if (e is List) {
      return toUnmodifiableListView (e) as dynamic /*=T*/;
// WARNING: The return type 'T (/home/myuser/dart/bwu_docker/lib/src/shared/json_util.dart)' is not a 'T (/usr/local/apps/dart/dart-sdk/lib/core/iterable.dart)', as defined by the method '' ([bwu_docker] lib/src/shared/json_util.dart:145)
// ERROR: Type check failed: toUnmodifiableListView(e) as T (T) is not of type T ([bwu_docker] lib/src/shared/json_util.dart:145)

    } else {
      return e as dynamic /*=T*/;
// WARNING: The return type 'T (/home/myuser/dart/bwu_docker/lib/src/shared/json_util.dart)' is not a 'T (/usr/local/apps/dart/dart-sdk/lib/core/iterable.dart)', as defined by the method '' ([bwu_docker] lib/src/shared/json_util.dart:147)
// ERROR: Type check failed: e as T (T) is not of type T ([bwu_docker] lib/src/shared/json_util.dart:147)
    }
  }));
}

Am I doing something wrong or is this not yet supported?

Related to

@leafpetersen
Copy link
Member

Explicitly instantiation of generic parameters doesn't work yet. @jmesserly is hard at work getting this in place. I believe that if you make the cast in the first line be to Iterable<int>, then upwards inference will infer the parameter type for you (amusingly we support inference before we support explicit instantiation).

I'm looking into the errors within the body of toUnmodifiableListView. Just one informational comment: by typing e explicitly as dynamic you are disabling inference for it. If you want it to be typed, you should either leave the type off of it, or else use dynamic/*=T*/ e.

@leafpetersen
Copy link
Member

Ok, the issue in the body of the method is the same - no explicit instantiation yet. The error message is confusing because the type parameter to the method (T) is named the same as the type parameter to the map method. If you change your type parameter to S, you'll see a clearer warning. The error message is bad, we need to fix - I think we should be giving it in terms of dynamic, rather than in terms of an uninstantiated type variable.

You can fix this in the short term by pulling the closure argument out into a local function with an explicit return type: this will allow inference to fill in the type on list.map for you.

Also, disregard my comment above about the closure parameter - I misunderstood the types on first read, and presumably you want that to be explicitly dynamic.

@leafpetersen
Copy link
Member

Short reproduction case of the issues to be fixed (besides supporting explicit instantiation):

void foo() {
  List list = null;

  // [warning] e (dynamic) will need runtime check to cast to type T
  list.map((e) => e);

  // [hint] The argument type '(dynamic) → T' cannot be assigned to the parameter type '(dynamic) → int'
  list.map((e) => 3);
}

The first one suggests that we are either not fully resolving, or are resolving too late, after we instantiate list.map with dynamic and are not getting rid of all uses of the type parameter.

The second is just a hint, and suggests that there is propagated type information that is not getting updated correctly when we infer a resolved type.

@jmesserly
Copy link

Ah ha. Think I know what's going on here. Upward inference runs too late, so downward inference (as well as the existing propagatedType inference for lambdas) sees the uninstantiated generic type on the generic method.

My explicit instantiation fix will include a fix for this. Early on, the unknown generic types will be "dynamic".

@jmesserly jmesserly self-assigned this Dec 4, 2015
@jmesserly jmesserly added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Dec 4, 2015
@jmesserly
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

4 participants