Sdk: [ddc] Constructors fail to assign default arguments in certain cases (possibly related to mixins)

[dkwingsmt]

Issue

In some cases, constructors do not assign the correct default arguments to properties. As a result, the property has a null value despite being non-nullable.

For example (psuedo-example for demonstration; for a full reproduction, see next section)

// NNBD enabled

abstract class PointerEvent {
  const PointerEvent({
    this.position = Offset.zero,
  });

  final Offset position;
}

class PointerAddedEvent extends PointerEvent with Mixin1, Mixin2 {
  const PointerAddedEvent();
}

expect(PointerAddedEvent().position, isNotNull); // AssertionError

This only occurs on Web. This issue is probably related to mixins, since it emerged in a change where a class is split into multiple mixins.

Reproduction

Stable reproduction:

• Checkout https://github.com/flutter/flutter/pull/63813, revision a764b46
• Run flutter test --platform=chrome test/gestures/events_test.dart
• Observation: Some tests fail (will explain below)
• Expectation: No tests fail

The test file does not fail without --platform=chrome. The test file does not fail before this PR.

Related issues

• Blocks https://github.com/flutter/flutter/pull/63813

[jason-simmons]

This can be reproduced by applying https://github.com/flutter/flutter/pull/63813 and running this Flutter test:

// @dart = 2.8

import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/gestures.dart';

void main() {
  test('events', () {
    const PointerAddedEvent added = PointerAddedEvent(
      position: Offset(20, 30),
    );
    print('added=$added delta=${added.delta}');
  });
}

delta will print as null even though the PointerEvent.delta field is non-nullable and the PointerEvent superclass constructor should initialize that field.

delta will be initialized correctly if you remove the mixins that were added to the PointerAddedEvent class in https://github.com/flutter/flutter/pull/63813

[nshahan]

@jonahwilliams The test flutter/test/gestures/events_test.dart has the language comment // @dart = 2.8 so I believe this is running in weak mode. While I agree it looks like we should have a default value, a null value appearing in a non-nullable field is technically allowed in weak mode. Has anyone reproduced this issue outside of the flutter test environment? I have not been able to yet.

[nshahan]

cc @dkwingsmt

[jason-simmons]

I still see the delta field initialized to null if I remove the // @dart = 2.8 comment from the simplified test in https://github.com/dart-lang/sdk/issues/43528#issuecomment-696977880

Running flutter test generates a .dart_tool/build/flutter_web/my_app/test/my_test.ddc.js build output.

With the mixins, the JavaScript PointerAddedEvent constructor sets delta to null:

    get C0() {
      return C0 = dart.const({
        __proto__: events.PointerAddedEvent.prototype,
        ...
        [PointerEvent_delta]: null,
        [PointerEvent_position]: C1 || CT.C1,

If I remove the mixins, then the delta initializer looks correct:

        [PointerEvent_delta]: C1 || CT.C1,
        [PointerEvent_position]: C2 || CT.C2,

[nshahan]

@jason-simmons Thanks for the pointer on where the js files get generated, that gives me a little more to go on.

Which mixins specifically did you remove and from which class?

[jason-simmons]

I removed the _PointerEventDescription and _CopyPointerAddedEvent mixins that were added to PointerAddedEvent in https://github.com/flutter/flutter/pull/63813

See https://github.com/flutter/flutter/pull/63813/files#diff-7b2ea4ecc4ea948de2b09c933dcc35d7R804

[nshahan]

@jason-simmons I able to repro the issue with the small test you posted when running flutter test.

added=PointerAddedEvent#62177(position: Offset(20.0, 30.0)) delta=null

When I run the exact same code with flutter run I can see that the delta is not null.

added=PointerAddedEvent#2633f(position: Offset(20.0, 30.0)) delta=Offset(0.0, 0.0)

Would you expect this? Is there a known difference between the test environment and just running as an app? Have you been able to reproduce this issue outside of the test environment?

[jason-simmons]

No, this is not expected behavior. In particular, the presence of the mixins should not affect how the constructor initializes delta. So it looks like something is going wrong during compilation of the test.

I have only been able to reproduce this using flutter test with a web/JavaScript target. flutter run for web and flutter test with native compilation work as expected.

[jonahwilliams]

flutter test with a web target is using build_web_compilers, while run uses the frontend_server

[nshahan]

I believe this is an issue with modular kernel compilation. I've created a minimal repro outside of flutter and filed another issue for the CFE team https://github.com/dart-lang/sdk/issues/43538.

[nshahan]

The fix to the CFE has landed in the Dart SDK.

@dkwingsmt can you verify the fix on your pending PR once 4025dee2f899c2118a6b3e767078f8c2ca57a515 has rolled into flutter?

[dkwingsmt]

The fix to the CFE has landed in the Dart SDK.

@dkwingsmt can you verify the fix on your pending PR once 4025dee has rolled into flutter?

Will do! Thanks you!

[dkwingsmt]

I can verify that this issue has been solved and https://github.com/flutter/flutter/pull/63813 has been merged.

Thank you very much for the fix!

[nshahan]

For anyone finding this issue in the future, the fix was picked to be included in Dart SDK v2.10.2 https://github.com/dart-lang/sdk/issues/43698.