Tests do not take normalization into consideration.

[crelier]

co19_2/Language/Generics/class_A03_t01
co19_2/Language/Generics/class_A03_t02

must take normalization into account:
https://github.com/dart-lang/language/blob/master/resources/type-system/normalization.md

The tests listed above will start failing after the backends implements normalization, e.g.

/====================================================================================\
| co19_2/Language/Generics/class_A03_t01 broke (Pass -> RuntimeError, expected Pass) |
\====================================================================================/

--- Command "vm" (took 01.000897s):
DART_CONFIGURATION=DebugX64NNBD out/DebugX64NNBD/dart --enable_asserts --enable-experiment=non-nullable --ignore-unrecognized-flags --packages=/b/s/w/ir/.packages /b/s/w/ir/tests/co19_2/src/Language/Generics/class_A03_t01.dart

exit code:
255

stderr:
Unhandled exception:
Expect.equals(expected: <FutureOr<dynamic>>, actual: <dynamic>) fails.
#0      _fail (file:///b/s/w/ir/tests/co19_2/src/Utils/expect.dart:21:5)
#1      Expect.equals (file:///b/s/w/ir/tests/co19_2/src/Utils/expect_common.dart:30:5)
#2      C.check (file:///b/s/w/ir/tests/co19_2/src/Language/Generics/class_A03_t01.dart:30:12)
#3      main (file:///b/s/w/ir/tests/co19_2/src/Language/Generics/class_A03_t01.dart:46:23)
#4      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:300:19)
#5      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:167:12)

--- Re-run this test:
python tools/test.py -n dartk-weak-asserts-linux-debug-x64 co19_2/Language/Generics/class_A03_t01

[eernstg]

@crelier, could you say a few words about how this situation arose? Is it a log from a buildbot or trybot?

[leafpetersen]

cc @munificent @athomas What's the process we're following for keeping legacy tests (e.g. language_2 or co19_2) opted out? Is this done via package_config.json? Is this already in place for co19_2?

[athomas]

I can see two parts of code that seem to be related to opting out co19_2 legacy tests: Analyzer: https://github.com/dart-lang/sdk/blob/38c7291e9e6ec7a22e87c7734bfbd3a38b075883/pkg/test_runner/lib/src/compiler_configuration.dart#L956 CFE: https://github.com/dart-lang/sdk/blob/5b76fc217295a4a62f2b89a96051d8b204c433ae/pkg/front_end/lib/src/fasta/source/source_library_builder.dart#L401

…

On Thu, May 7, 2020, 17:24 Leaf Petersen ***@***.***> wrote: cc @munificent <https://github.com/munificent> @athomas <https://github.com/athomas> What's the process we're following for keeping legacy tests (e.g. language_2 or co19_2) opted out? Is this done via package_config.json? Is this already in place for co19_2? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#531 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEYRJBZMMNBCQW6R3FZOQADRQLHCZANCNFSM4KYEUPBA> .

[crelier]

@crelier, could you say a few words about how this situation arose? Is it a log from a buildbot or trybot?

The original stack trace of this issue was obtained from a build bot.
I see that the fix to this issue was reverted yesterday without causing the failure. That is because of this CL: https://dart-review.googlesource.com/c/sdk/+/139955, which is normalizing FutureOr when reading from the kernel file.

At the time this issue was filed, the FutureOr instantiated via the type parameter T would get normalized to dynamic by the VM at runtime. However, the literal FutureOr provided via the kernel file was not normalized.

[eernstg]

I think we have two confusing elements here:

1. A test in co19_2 is executed with --enable-experiment=non-nullable.

2. With CL 139955, normalization is performed more aggressively by the VM at run time than at least I would expect.

Ad 1, I believe the source code locations mentioned by @athomas here support the assumption that tests in co19_2 would simply never be executed with --enable-experiment=non-nullable. Nevertheless, it did apparently occur. If this is going to be a normal occurrence then we may need to edit all the tests in co19_2 to say // @dart = 2.6 or something like that. But it was clearly the intention that the co19_2 tests should be able to remain unchanged, because they are intended to verify that developers out there can continue to work with their legacy code without doing such things as adding // @dart = 2.6 to every library.

Ad 2, I think we would need to discuss whether it is OK for a Dart execution to apply type normalization eagerly. The fact that we have a test failure here illustrates that it is an observable property, even though it does not change subtype relationships and (if I understood that correctly) only affects types obtained by inference or instantiation to bound.

So will we run co19_2 tests with null-safety enabled, and is CL 139955 a breaking change?

@leafpetersen, @athomas, @crelier?

[crelier]

We consider type normalization as a "pre-nnbd breaking change". See the related sdk issue and language issue. Therefore, the result should be independent of the value of the --enable-experiment=non-nullable flag.

Regarding the normalization itself, its spec states that "... normal forms for types could be eagerly or lazy computed and cached". The VM eagerly normalizes types, and therefore applies normalization when reading a kernel file on types left unnormalized by the CFE.

[eernstg]

Thanks! I was worried about the implications of performing normalization in a set of situations that isn't specified, but I think the differences are sufficiently restricted to ensure a well-defined semantics after all.

@crelier wrote:

We consider type normalization as a "pre-nnbd breaking change".

Right, dart-lang/sdk#40633 announces that type objects (that I interpret to mean instances of Type or a subtype thereof which are obtained as reifications of Dart types) will be normalized. That's benign, in the sense that we have specification language saying that operator == on these objects must normalize the operands as part of the comparison, and it seems very likely that it is not observable whether this normalization occurs early or late. So that is basically just an optimization.

dart-lang/sdk#40633 also mentions that .runtimeType as implemented in Object will return a normalized result. This is benign from a specification point of view because .runtimeType is unspecified, and it is very likely to be non-breaking in practice, in a world where we already have changed said operator == to use normalization. So I'm not worried about that either.

However, we obviously can't use normalization arbitrarily early, e.g., we do want the following program to have a compile-time error:

import 'dart:async';
void main() {
  int i = null as FutureOr;
}

If we normalize FutureOr to dynamic up front then this will be accepted, but we want the type dynamic to be treated specially in a restrictive manner, and we should not silently eliminate the compile-time error and accept a program like this, just because FutureOr is "so similar" to dynamic.

In particular, I was worried when I saw this because it wasn't entirely obvious to me how early the normalization could occur, and what this could imply:

At the time this issue was filed, the FutureOr instantiated via the
type parameter T would get normalized to dynamic by the VM at
runtime. However, the literal FutureOr provided via the kernel file
was not normalized.

But even though the application of normalization may be considerably more implementation specific than I had expected (for instance, dart2js and the vm may do it differently), I believe it is safe, because we're only talking about eagerness which occurs in backends, and it seems very likely that this will not be observable.