dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.27k stars 1.58k forks source link

Analyzer/CFE type error inconsistency #48687

Open nshahan opened 2 years ago

nshahan commented 2 years ago

The analyzer doesn't report any errors for the following code:

main() {
  print('hello world');
}

typedef T FnType<T>(arguments);

class C {}

FnType fn<S extends C>(S s) => throw 'unimplemented';

FnType gn<R extends C?>(R r) {
  if (r == null) throw 'null!';

  return fn(r);
}

The CFE reports:

test_05.dart:14:10: Error: Inferred type argument 'R' doesn't conform to the bound 'C' of the type variable 'S' on 'fn'.
 - 'C' is from 'test_05.dart'.
Try specifying type arguments explicitly so that they conform to the bounds.
  return fn(r);
         ^
test_05.dart:9:11: Context: This is the type variable whose bound isn't conformed to.
FnType fn<S extends C>(S s) => throw 'unimplemented';

I don't quite know which tool is correct but neither tool reports an error if I change the signature of gn() by moving the nullability from the type parameter bound C to the type of the method parameter R:

FnType gn<R extends C>(R? r)

This inconsistency could be related to the null safety migration tool recommending an invalid migration on the same code that motivated this example: https://github.com/dart-lang/sdk/issues/47913

cc @leafpetersen

eernstg commented 2 years ago

Here are a couple of adjustments to the example, to clarify the situation:

class C {
  void foo() {}
}

FnType gn<R extends C?>(R r) {
  if (r == null) throw 'null!';
  r.foo(); // OK, `r` has type `R & C`.
  return fn(r);
}

So we're invoking the method foo on r in gn, and that's possible because the type of r has been promoted from R to R & C, and in particular it's known to have the members of C.

That promotion is specified in this section, in the item about operator ==, in subitems about the null literal, which are using promoteToNonNull.

However, the type R & C is a purely static type and there will not be a run-time representation of it anywhere, so there's no way we could infer fn(r) to mean fn<R & C>(r). This is handled by "erasing" R & C to R, which yields fn<R>(r), but that is not type correct because we can't show that R <: C (we only have R <: C?).

However, I couldn't find the above mentioned erasure of R & C to R in spec documents. @leafpetersen, you wrote the following in https://github.com/dart-lang/sdk/issues/30832#issue-259285024:

The simplest fix for this, I think, is to just make inference strip off the promotion when inferring type type variables or lambda parameter/return types

If we agree (and specify, if it is indeed not already specified) that the erasure is the correct step to take in this situation, the analyzer should report the same error as the CFE.

srawlins commented 1 year ago

Our behavior is unchanged.