CFE: Constants evaluated incorrectly in legacy mode

[leafpetersen]

The program below should print true when run in legacy (weak) checking mode. In strong checking mode, the first constant should evaluate to false and the second should be a compile time error. Currently, the CFE seems to evaluate both constants with the strong semantics regardless of whether --null-safety or --no-null-safety is passed.

const isLegacySubtyping = <Null>[] is List<int>;
const assertLegacySubtyping = <Null>[] as List<int>;

void main() {
  print(isLegacySubtyping);
}

cc @nshahan @sigmundch

[nshahan]

Looks like @rakudrama already fixed the use in dart2js.
https://dart-review.googlesource.com/c/sdk/+/144673

[sigmundch]

@leafpetersen how were running the example under the two modes? I'm curious if this is a plumbing issue or a CFE issue. In particular, in DDC we don't plumb the flag yet, so we are always evaluating constants in legacy mode. Dart2js does provide the flag to the CFE, but we have a direct constant we can use so Stephen updated the patch file. We can do the same kind of change in DDC as well.

[leafpetersen]

@sigmundch See below. It seems unlikely to be a plumbing error since:

• The compile time error is presumably issued by the CFE
• I believe @nshahan sees DDC getting the constant value false from the CFE for the first constant.

leafp-macbookpro:sdk leafp$ ~/src/dart-repo/sdk/xcodebuild/ReleaseX64NNBD/dart-sdk/bin/dart  --enable-experiment=non-nullable --null-safety ~/tmp/test.dart
../../../tmp/test.dart:2:40: Error: Constant evaluation error:
const assertLegacySubtyping = <Null>[] as List<int>;
                                       ^
../../../tmp/test.dart:2:40: Context: Expected constant '<Null?>[]' to be of type 'List<int>', but was of type 'List<Null?>'.
 - 'List' is from 'dart:core'.
const assertLegacySubtyping = <Null>[] as List<int>;
                                       ^
../../../tmp/test.dart:2:7: Context: While analyzing:
const assertLegacySubtyping = <Null>[] as List<int>;
      ^
leafp-macbookpro:sdk leafp$ ~/src/dart-repo/sdk/xcodebuild/ReleaseX64NNBD/dart-sdk/bin/dart  --enable-experiment=non-nullable --no-null-safety ~/tmp/test.dart
../../../tmp/test.dart:2:40: Error: Constant evaluation error:
const assertLegacySubtyping = <Null>[] as List<int>;
                                       ^
../../../tmp/test.dart:2:40: Context: Expected constant '<Null?>[]' to be of type 'List<int>', but was of type 'List<Null?>'.
 - 'List' is from 'dart:core'.
const assertLegacySubtyping = <Null>[] as List<int>;
                                       ^
../../../tmp/test.dart:2:7: Context: While analyzing:
const assertLegacySubtyping = <Null>[] as List<int>;
      ^

[nshahan]

I just sent a workaround for review because it's a quick fix but I will follow up with a change that uses the flag we have in the runtime. I've been meaning to add some protection against it being changed at runtime.

https://dart-review.googlesource.com/c/sdk/+/144815

[sigmundch]

@leafpetersen - thanks for the clarification, makes sense. I guess the VM is using the agnostic platform file, so it also seems plausible that it is not properly converting the agnostic constant to a weak constant :-/

@nshahan - thanks, thinking about the change I realized that we have one additional challenge in our build configuration: we currently only build one .dill file for the sdk. To properly support either mode as a compile-time flag, we may need also to ship 2 .dill files in the future.

[leafpetersen]

hanks for the clarification, makes sense. I guess the VM is using the agnostic platform file, so it also seems plausible that it is not properly converting the agnostic constant to a weak constant :-/

Note that my repro above is not in the SDK. Does the VM use agnostic constants everywhere?

[nshahan]

cc @johnniwinther