Skip to content

[cfe] Implement support for generic function instantiation of existing function objects in constants #47154

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
eernstg opened this issue Sep 8, 2021 · 8 comments
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.

Comments

@eernstg
Copy link
Member

eernstg commented Sep 8, 2021

This is the CFE specific issue for #47150, which has the details.

@eernstg
Copy link
Member Author

eernstg commented Sep 10, 2021

There is actually still an issue with the CFE which is associated with potentially constant type expressions:

// A potentially constant type expression is supported for `as` (and `is`)
class A<X> {
  final List<X> x;
  const A(x): x = x is List<X> ? x : x as List<X>;
}

void m<X>(X x) {}

// Generic function instantiation to a type parameter is supported implicitly.
class B<X> {
  final void Function(X) f;
  const B(): f = m;
}

// But it is not supported explicitly.
class C<X> {
  final f;
  const C(): f = m<X>; // Error, but should be accepted.
}

void main() {
  const A<int>(<int>[1]); // OK.
  const b = B<String>(); // OK.
  print(b.f.runtimeType); // OK: 'String => void'.
  const c = C<String>(); // Compile-time error in `C`, but should be accepted when it works.
  print(c.f.runtimeType); // (Never executed, so we don't know).
}

The tests in https://dart-review.googlesource.com/c/sdk/+/213047 do include similar cases.

@eernstg eernstg reopened this Sep 10, 2021
@johnniwinther
Copy link
Member

Do we also support type variables in type literals as potentially constant, and if so, should we also support type variables as potentially constant type literal?

class Class<T> {
  final field1;
  final field2;
  const Class() 
    : this.field1 = Class<T>, // Should this be allowed now?
      this.field2 = T; // Should this also be allowed now?
}

@eernstg
Copy link
Member Author

eernstg commented Sep 13, 2021

Given that we do support the implicit generic instantiation (where a function object is used to initialize an instance variable), it seems inconsistent to rule out the explicit form. Here's the implicit one:

X id<X>(X x) => x;

class A<X> {
  final X Function(X) f;
  const A(): f = id;
}

void main() {
  const c = A<int>();
  print(c.f.runtimeType); // `(int) => int`.
}

The new section in the constructor-tearoffs specification says that e<T> is a potentially constant expression (as required for an expression in an initializer list) under some syntactic criteria (as usual, we don't want to check any static semantic properties during a 'potentially constant' check). So this allows for the type argument of the enclosing class to occur as the T.

When we encounter a <constObjectExpression>, we substitute actual arguments for formal parameters, and the resulting expression must be constant. Given that the actual type arguments to the constant object expression must be constant type expressions, I believe that this case is included.

However, if it gives rise to substantial refactorings or other implementation work then we'd need to reconsider.

@johnniwinther
Copy link
Member

I wasn't thinking of the implicit/explicit instantiations (which your example is about) but of the type literals that use type variables.

In fixing the compile-time error we emitted for the type variable occurring in an explicit instantiation, I just happened to touch the similar code for type variable type literals (and with that the newly supported generic type literals which might now contain type variables within them) and wondered whether we should support these in potentially context expressions.

@eernstg
Copy link
Member Author

eernstg commented Sep 14, 2021

You're right, type literals are covered in the same way by the same part of the constructor-tearoffs specification, so this.field1 = Class<T> should be allowed. It doesn't cover a lone identifier, though, so this.field2 = T is not supported.

That's silly, because we could just use typedef F<X> = X; and make it this.field2 = F<T>.

I think this means that the consistent approach would be to allow both.

@johnniwinther
Copy link
Member

@eernstg Will you ensure that this.field2 = T will be spec'ed to be supported as part of the constructor-tearoff feature?

@eernstg
Copy link
Member Author

eernstg commented Sep 16, 2021

Yes, that would have happened already several days ago except for that influenza (or whatever it was).

@eernstg
Copy link
Member Author

eernstg commented Sep 17, 2021

Cf. dart-lang/language#1860.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.
Projects
None yet
Development

No branches or pull requests

2 participants