Skip to content

Commit 6c9dbb3

Browse files
stereotype441Commit Queue
authored and
Commit Queue
committed
Front end: fix promotion of fields accessed through mixin applications.
When a field is declared in a mixin, the front end creates a synthetic getter in the mixin application class that gets the value of the mixed in field. So if a piece of code accesses the mixed in field through the mixin application class rather than through the mixin directly, the resolved member is the synthetic getter rather than a field. In order to ensure that the field remains promotable even if it is accessed through the mixin application, the logic in `OperationsCfe.isPropertyPromotable` needs to be changed so that it doesn't treat these synthetic getters as non-promotable. The old logic was essentially this: 1. If the property is not private, it's not promotable. 2. Otherwise, if the property is listed in `FieldNonPromotabilityInfo.fieldNameInfo`, it's not promotable. (This happens either if the property is not promotable for an intrinsic reason, such as being a non-final field or a concrete getter, or if it has the same name as a non-promotable property elsewhere in the library). 3. Otherwise, if the property is a getter that was lowered from an abstract field, it's promotable. 4. Otherwise, if the property is a getter that was lowered from a late field, it's promotable. 5. Otherwise, the property isn't promotable. (This was intended to cover the case where the property is an abstract getter declaration). (Although conditions 3 and 4 were tested first, since they are more efficient to test). It turns out that once conditions 1-2 have been ruled out, the property must have been declared as a method (which is being torn off), a private abstract getter, or a (possibly abstract) non-external private final field. Of these three possibilities, only the last is promotable. So this can be simplified to: (conditions 1-2 as above) 3. Otherwise, if the property is a method tear-off, it's not promotable. 4. Otherwise, if the property is an abstract getter, it's not promotable. 5. Otherwise, the property is promotable. This makes the logic easier to follow, since conditions 1-4 are now all reasons for non-promotability (rather than a mix of promotability and non-promotability reasons). It also conveniently addresses the problem with fields accessed through mixin applications, since they aren't excluded by any of conditions 1-4. (We still test conditions 3 and 4 first, since they are more efficient to test.) Fixes #53742. Fixes #53617. Fixes #53436. Change-Id: I64df269c2a4a0714f9be239d832b61f4fb6a1a43 Bug: #53742 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330168 Reviewed-by: Nate Bosch <[email protected]> Reviewed-by: Chloe Stefantsova <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 51dde06 commit 6c9dbb3

File tree

4 files changed

+119
-6
lines changed

4 files changed

+119
-6
lines changed

pkg/front_end/lib/src/fasta/kernel/hierarchy/class_member.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,8 @@ class SynthesizedInterfaceMember extends SynthesizedMember {
410410
library.forwardersOrigins
411411
..add(stub)
412412
..add(canonicalMember);
413+
stub.isAbstractFieldAccessor =
414+
canonicalMember.isAbstractFieldAccessor;
413415
}
414416
_member = stub;
415417
_covariance = combinedMemberSignature.combinedMemberSignatureCovariance;

pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -514,12 +514,12 @@ class OperationsCfe
514514
this.fieldNonPromotabilityInfo;
515515
if (fieldNonPromotabilityInfo == null) return false;
516516
if (property is Procedure) {
517-
if (property.isAbstractFieldAccessor || property.isLoweredLateField) {
518-
// Property was declared as a field; it was lowered to a getter or
519-
// getter/setter pair. So for field promotion purposes treat it as a
520-
// field.
521-
} else {
522-
// We don't promote methods or explicit getters.
517+
if (!property.isAccessor) {
518+
// We don't promote methods.
519+
return false;
520+
}
521+
if (property.isAbstract && !property.isAbstractFieldAccessor) {
522+
// We don't promote direct references to abstract getter declarations.
523523
return false;
524524
}
525525
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Tests that field promotion logic properly handles promotable abstract fields
6+
// declared in mixins.
7+
8+
// This test exercises both syntactic forms of creating mixin applications
9+
// (`class C = B with M;` and `class C extends B with M {}`), since these are
10+
// represented differently in the analyzer.
11+
12+
// This test exercises both the scenario in which the mixin declaration precedes
13+
// the application, and the scenario in which it follows it. This ensures that
14+
// the order in which the mixin declaration and application are analyzed does
15+
// not influence the behavior.
16+
17+
// SharedOptions=--enable-experiment=inference-update-2
18+
19+
import '../static_type_helper.dart';
20+
21+
abstract class C1 = Object with M;
22+
23+
abstract class C2 extends Object with M {}
24+
25+
mixin M {
26+
abstract final int? _field;
27+
}
28+
29+
abstract class C3 = Object with M;
30+
31+
abstract class C4 extends Object with M {}
32+
33+
void test(C1 c1, C2 c2, C3 c3, C4 c4) {
34+
if (c1._field != null) {
35+
c1._field.expectStaticType<Exactly<int>>();
36+
}
37+
if (c2._field != null) {
38+
c2._field.expectStaticType<Exactly<int>>();
39+
}
40+
if (c3._field != null) {
41+
c3._field.expectStaticType<Exactly<int>>();
42+
}
43+
if (c4._field != null) {
44+
c4._field.expectStaticType<Exactly<int>>();
45+
}
46+
}
47+
48+
main() {}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Tests that field promotion logic properly handles promotable fields declared
6+
// in mixins.
7+
8+
// This test exercises both syntactic forms of creating mixin applications
9+
// (`class C = B with M;` and `class C extends B with M {}`), since these are
10+
// represented differently in the analyzer.
11+
12+
// This test exercises both the scenario in which the mixin declaration precedes
13+
// the application, and the scenario in which it follows it. This ensures that
14+
// the order in which the mixin declaration and application are analyzed does
15+
// not influence the behavior.
16+
17+
// SharedOptions=--enable-experiment=inference-update-2
18+
19+
import '../static_type_helper.dart';
20+
21+
class C1 = Object with M;
22+
23+
class C2 extends Object with M {}
24+
25+
mixin M {
26+
final int? _nonLate = 0;
27+
late final int? _late = 0;
28+
}
29+
30+
class C3 = Object with M;
31+
32+
class C4 extends Object with M {}
33+
34+
void test(C1 c1, C2 c2, C3 c3, C4 c4) {
35+
if (c1._nonLate != null) {
36+
c1._nonLate.expectStaticType<Exactly<int>>();
37+
}
38+
if (c1._late != null) {
39+
c1._late.expectStaticType<Exactly<int>>();
40+
}
41+
if (c2._nonLate != null) {
42+
c2._nonLate.expectStaticType<Exactly<int>>();
43+
}
44+
if (c2._late != null) {
45+
c2._late.expectStaticType<Exactly<int>>();
46+
}
47+
if (c3._nonLate != null) {
48+
c3._nonLate.expectStaticType<Exactly<int>>();
49+
}
50+
if (c3._late != null) {
51+
c3._late.expectStaticType<Exactly<int>>();
52+
}
53+
if (c4._nonLate != null) {
54+
c4._nonLate.expectStaticType<Exactly<int>>();
55+
}
56+
if (c4._late != null) {
57+
c4._late.expectStaticType<Exactly<int>>();
58+
}
59+
}
60+
61+
main() {
62+
test(C1(), C2(), C3(), C4());
63+
}

0 commit comments

Comments
 (0)