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.3k stars 1.59k forks source link

New `co19/LanguageFeatures/nnbd/const_evaluation_A10_t01` test fails on VM in weak mode #52511

Open mkustermann opened 1 year ago

mkustermann commented 1 year ago

See log:

Unhandled exception:
Expect.identical(expected: <Never>, actual: <Never>) fails.
#0      _fail (file:///b/s/w/ir/cache/builder/sdk/tests/co19/src/Utils/expect.dart:18:5)
#1      Expect.identical (file:///b/s/w/ir/cache/builder/sdk/tests/co19/src/Utils/expect_common.dart:52:7)
#2      testXExtendsT1 (file:///b/s/w/ir/cache/builder/sdk/tests/co19/src/LanguageFeatures/nnbd/const_evaluation_A10_t01.dart:52:10)
#3      main (file:///b/s/w/ir/cache/builder/sdk/tests/co19/src/LanguageFeatures/nnbd/const_evaluation_A10_t01.dart:111:3)
#4      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:296:19)
#5      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:189:12)

--- Re-run this test:
python3 tools/test.py -n dartk-weak-asserts-linux-debug-x64 co19/LanguageFeatures/nnbd/const_evaluation_A10_t01

It seems this is due to weak mode, as:

foo<T extends Never>() {
  if (!identical(T, Never)) throw 'bar';
}
main() {
  foo();
}

Passes by-default

% out/ReleaseX64/dart test.dart
% 

Fails in non sound null safety

% out/ReleaseX64/dart --no-sound-null-safety  test.dart
Unhandled exception:
bar
#52480
...

/cc @alexmarkov

alexmarkov commented 1 year ago

T is Never according to normalization rules, but type literal Never is actually Never* in non-sound mode (due to constant erasure), so they are not identical.

Kernel AST:

  static method foo<T extends Never>() → dynamic {
    core::print(foo::foo::T);
    core::print(#C1);
    if(!core::identical(foo::foo::T, #C1))
      throw "bar";
  }
  static method main() → dynamic {
    foo::foo<Never>();
  }

  #C1 = TypeLiteralConstant(Never*)

Either this test is incorrect (cc @leafpetersen) or this is a front-end bug (cc @johnniwinther).

leafpetersen commented 1 year ago

I think we specified this identity to hold: https://github.com/dart-lang/language/blob/main/accepted/2.12/nnbd/feature-specification.md#type-literals . @eernstg might check me there.

That said, I'm not sure this corner of weak mode semantics is a high priority to fix at this point.

eernstg commented 1 year ago

Right, the priority is a bit low for tests that are going to go away soon.

However, I'm actually arriving at a more vague conclusion: identical can return true or false in this case.

foo<T extends Never>() {
  if (!identical(T, Never)) throw 'bar';
}
main() {
  foo();
}

The library is null safe (at least, this test has no @dart = ... specifier), so Never means Never, not Null.

Never is a constant expression. It should be canonicalized, but it is unspecified which canonical representation is chosen for the set of reified types that are equal according to the type equality operation.

So it could evaluate to the Type that reifies Never or Never*, none of those is a violation of the specified behavior.

The invocation foo() in main would be inferred as foo<Never>().

This is then the run-time value of T when foo is executed. T is not a constant expression, and hence there is no normalization step when T is evaluated to a reified type, which would then be the Type for Never.

So, as far as I can see, both true and false are acceptable results from identical(T, Never). On the other hand, T == Never should evaluate to true (and presumably it does not matter which value is passed to T at the call site, because T is normalized to Never no matter what).

Perhaps the test should simply be adjusted to test if (T != Never) throw 'bar';.