The switch from Bytecode to AST has revealed differences in behavior which were approved

[mkustermann]

It seems at least the following failures were approved after the change from Bytecode -> AST has landed in 8bf424e / cl/120720.

language_2/regress_29025_test: Crash
co19_2/LanguageFeatures/Instantiate-to-bound/class/static/class_typedef_l1_t04/none: DartkCrash
co19_2/LanguageFeatures/Instantiate-to-bound/class/static/class_typedef_l2_t08/none: DartkCrash
language_2/covariant_subtyping_with_mixin_test: RuntimeError
language_2/type_alias_equality_test/03: RuntimeError
language_2/vm/bool_check_stack_traces_test/02: RuntimeError
service/evaluate_activation_test/instance: RuntimeError
vm/dart/il_round_trip_deserialization_test/5: Crash
vm/dart/il_round_trip_deserialization_test/7: Crash

These failures should be investigated and triaged.

(The failures have been approved - but I haven't found any github issue for it!)

/cc @mraleph @a-siva @alexmarkov

[alexmarkov]

These failures are among hundreds of other approved test failures (see test results dashboard for the full list), and they were failing before bytecode was enabled. Some of them are even mentioned in status files and originate before status-file free workflow. They just happened to be fixed by new bytecode pipeline due to various reasons (for example, bypassing certain bugs in AST serialization or flow graph construction). Is there any particular reason to look at them and not other failures, especially given that they will be fixed by switching to the new bytecode pipeline?

[mkustermann]

Just because they are seemingly passing on bytecode pipeline doesn't mean that there isn't a bug.

For example the vm/dart/il_round_trip* tests: They test functionality for serializing IR graphs out and reading them back in. They should not crash. The failures should be analyzed and triaged and not get ignored.

[alexmarkov]

Here is my understanding why some of these tests fail with AST but pass with bytecode:

language_2/regress_29025_test - most likely related to finalization of recursive types. In bytecode mode most of the type finalization machinery is skipped as types are finalized during bytecode generation.

co19_2/LanguageFeatures/Instantiate-to-bound/class/static/class_typedef_l1_t04/none, co19_2/LanguageFeatures/Instantiate-to-bound/class/static/class_typedef_l2_t04/none, co19_2/LanguageFeatures/Instantiate-to-bound/class/static/class_typedef_l2_t08/none - crash during kernel AST serialization. Crashed code is not used when bytecode is serialized.

language_2/covariant_subtyping_with_mixin_test - problem with how AST-based flow graph builder generates parameter type checks in forwarding stubs, tracked in #34321. Bytecode generator correctly handles this case.

language_2/type_alias_equality_test/03 - this test is for equality of function types. The underlying problem is tracked in #32783. It passes in bytecode as bytecode pipeline is more likely to canonicalize types.

language_2/vm/bool_check_stack_traces_test/02 - AST-based flow graph builder does not correctly assign token position in AssertBoolean check in the code like if (null). Bytecode generator is able to handle this case.

service/evaluate_activation_test/instance - this test is related to expression evaluation inside closures and tracked in #20047. This test passes in bytecode mode because this is captured in bytecode mode and it's available for expression evaluation.

vm/dart/il_round_trip_deserialization_test/5, vm/dart/il_round_trip_deserialization_test/7 - tracked in #36882. There are no comments saying that underlying problem is fixed after the comment #36882 (comment) about these failures. My guess is that flow graph constructed from bytecode is sufficiently different from the one constructed from AST and it doesn't trigger a bug in serialization.

[mkustermann]

Thank you very much for the analysis, @alexmarkov !

co19_2/LanguageFeatures/Instantiate-to-bound/class/static/class_typedef_l1_t04/none,
crash during kernel AST serialization. Crashed code is not used when bytecode is serialized.

We should investigate this issue. I've filed #38812 for the CFE team. @johnniwinther already fixed the bug. Do we know why the bytecode compiler didn't barf when seeing a UnknownType?

vm/dart/il_round_trip_deserialization_test/7 tracked in #36882
There are no comments saying that underlying problem is fixed

Yes, but if the problem doesn't reproduce anymore with bytecode on-by-default it's hard to know the bug is still there. Attempting to reproduce it might fail and the issue might get closed.

At least we have to ping the github issue to say that turning bytecode on will make the issue disappear but the bug is probably still there.

[alexmarkov]

Do we know why the bytecode compiler didn't barf when seeing a UnknownType?

From the call stack of the crash it looks like UnknownType happened in the typedef declaration. typedef declarations are not serialized in bytecode - only the underlying function types.