Skip to content

Pattern match missing inference and generic method instantiation #51871

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
nshahan opened this issue Mar 27, 2023 · 4 comments
Closed

Pattern match missing inference and generic method instantiation #51871

nshahan opened this issue Mar 27, 2023 · 4 comments
Assignees
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.

Comments

@nshahan
Copy link
Contributor

nshahan commented Mar 27, 2023

A missing generic instantiation is causing a compile time error here:

void testRelational() {
const c = id;
// Instantiates based on the context type of the "<" method parameter type.
if (Compare() case < c) {
Expect.fail('"<" should receive instantiation, not generic function.');
} else {
// OK.
}
}

org-dartlang-app:/tests/language/patterns/implicit_instantiation_test.dart:36:24: Error: The argument type 'T Function<T>(T)' can't be assigned to the parameter type 'int Function(int)'.
  if (Compare() case < c) {
                       ^

In a discussion with @munificent he mentioned that this pattern match should have the same downward inference and generic instantiation as this similar code that doesn't contain any patterns and runs without any errors:

T id<T>(T t) => t;

typedef IntFn = int Function(int);
typedef TFn = T Function<T>(T);

class Compare {
  operator <(IntFn f) {
    print(f.runtimeType);
    return f is TFn;
  }
}

main() {
  const c = id;
  Compare() < c;
}

cc @johnniwinther

@nshahan nshahan added the legacy-area-front-end Legacy: Use area-dart-model instead. label Mar 27, 2023
@munificent
Copy link
Member

Yeah, my suspicion is that this is just a bug where the context type of the relational method isn't be passed down for downwards inference to the relational pattern's constant operand.

I should note that this is a bug, but it's a pretty trivial one—almost no real-world relational or equality operators have a type annotation for the RHS that would lead to any interesting type inference.

@chloestefantsova
Copy link
Contributor

@munificent I have some questions about generalizing that kind of coercion to other pattern-related spots. Should the same be performed in switch expressions, for example, in the following program?

T id<T>(T t) => t;

typedef IntFn = int Function(int);
typedef TFn = T Function<T>(T);

class Compare {
  operator <(IntFn f) => f is TFn;
}

test1() {
  const c = id;
  return switch(Compare()) {
    < c => throw "Error",
    _ => "OK"
  };
}

main() {
  test1();
}

Generalizing even more, should all types of coercions be enabled for the operands of all relational patterns in all pattern-related language constructs? If so, the following CFE test, which currently is compiled without any errors, will be rejected because of the attempt to coerce string s to num.

https://github.com/dart-lang/sdk/blob/main/pkg/front_end/testcases/patterns/relational_assignable.dart

@eernstg
Copy link
Member

eernstg commented Mar 30, 2023

should all types of coercions be enabled for the operands of all relational patterns in all pattern-related language constructs?

We do not have a specification of the precise set of locations where coercions are applied (they are specified to be applicable everywhere, based on the context type and the type of the expression only, but they have always been implemented such that they were only applied in a smaller set of locations aka assignment locations). This means that we can't find anything about not applying coercions outside those assignment locations in the language specification or any other specification document.

However, I believe that we will specify this; in particular, there is no support in the language team for changing Function f = callableObject..memberOnCallableObject(); into Function f = (callableObject.call)..memberOnCallableObject(); and then reporting an error (because the torn-off function object doesn't have that method). In other words, we're almost certainly going to specify something like assignable locations and say that coercions only occur there, we're not likely to change the implementation to do exactly what is currently specified.

@chloestefantsova
Copy link
Contributor

Thank you for the clarification, Erik, and for confirming that the existing CFE unit test should indeed be treated as expecting a compile-time error.

I'm working on the fix at https://dart-review.googlesource.com/c/sdk/+/292001.

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

4 participants