mirror of
https://github.com/dart-lang/sdk
synced 2024-09-16 03:36:59 +00:00
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: https://github.com/dart-lang/sdk/issues/53742 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330168 Reviewed-by: Nate Bosch <nbosch@google.com> Reviewed-by: Chloe Stefantsova <cstefantsova@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
parent
51dde069b2
commit
6c9dbb35b5
|
@ -410,6 +410,8 @@ class SynthesizedInterfaceMember extends SynthesizedMember {
|
|||
library.forwardersOrigins
|
||||
..add(stub)
|
||||
..add(canonicalMember);
|
||||
stub.isAbstractFieldAccessor =
|
||||
canonicalMember.isAbstractFieldAccessor;
|
||||
}
|
||||
_member = stub;
|
||||
_covariance = combinedMemberSignature.combinedMemberSignatureCovariance;
|
||||
|
|
|
@ -514,12 +514,12 @@ class OperationsCfe
|
|||
this.fieldNonPromotabilityInfo;
|
||||
if (fieldNonPromotabilityInfo == null) return false;
|
||||
if (property is Procedure) {
|
||||
if (property.isAbstractFieldAccessor || property.isLoweredLateField) {
|
||||
// Property was declared as a field; it was lowered to a getter or
|
||||
// getter/setter pair. So for field promotion purposes treat it as a
|
||||
// field.
|
||||
} else {
|
||||
// We don't promote methods or explicit getters.
|
||||
if (!property.isAccessor) {
|
||||
// We don't promote methods.
|
||||
return false;
|
||||
}
|
||||
if (property.isAbstract && !property.isAbstractFieldAccessor) {
|
||||
// We don't promote direct references to abstract getter declarations.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,48 @@
|
|||
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
// Tests that field promotion logic properly handles promotable abstract fields
|
||||
// declared in mixins.
|
||||
|
||||
// This test exercises both syntactic forms of creating mixin applications
|
||||
// (`class C = B with M;` and `class C extends B with M {}`), since these are
|
||||
// represented differently in the analyzer.
|
||||
|
||||
// This test exercises both the scenario in which the mixin declaration precedes
|
||||
// the application, and the scenario in which it follows it. This ensures that
|
||||
// the order in which the mixin declaration and application are analyzed does
|
||||
// not influence the behavior.
|
||||
|
||||
// SharedOptions=--enable-experiment=inference-update-2
|
||||
|
||||
import '../static_type_helper.dart';
|
||||
|
||||
abstract class C1 = Object with M;
|
||||
|
||||
abstract class C2 extends Object with M {}
|
||||
|
||||
mixin M {
|
||||
abstract final int? _field;
|
||||
}
|
||||
|
||||
abstract class C3 = Object with M;
|
||||
|
||||
abstract class C4 extends Object with M {}
|
||||
|
||||
void test(C1 c1, C2 c2, C3 c3, C4 c4) {
|
||||
if (c1._field != null) {
|
||||
c1._field.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c2._field != null) {
|
||||
c2._field.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c3._field != null) {
|
||||
c3._field.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c4._field != null) {
|
||||
c4._field.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
}
|
||||
|
||||
main() {}
|
63
tests/language/inference_update_2/field_in_mixin_test.dart
Normal file
63
tests/language/inference_update_2/field_in_mixin_test.dart
Normal file
|
@ -0,0 +1,63 @@
|
|||
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
// Tests that field promotion logic properly handles promotable fields declared
|
||||
// in mixins.
|
||||
|
||||
// This test exercises both syntactic forms of creating mixin applications
|
||||
// (`class C = B with M;` and `class C extends B with M {}`), since these are
|
||||
// represented differently in the analyzer.
|
||||
|
||||
// This test exercises both the scenario in which the mixin declaration precedes
|
||||
// the application, and the scenario in which it follows it. This ensures that
|
||||
// the order in which the mixin declaration and application are analyzed does
|
||||
// not influence the behavior.
|
||||
|
||||
// SharedOptions=--enable-experiment=inference-update-2
|
||||
|
||||
import '../static_type_helper.dart';
|
||||
|
||||
class C1 = Object with M;
|
||||
|
||||
class C2 extends Object with M {}
|
||||
|
||||
mixin M {
|
||||
final int? _nonLate = 0;
|
||||
late final int? _late = 0;
|
||||
}
|
||||
|
||||
class C3 = Object with M;
|
||||
|
||||
class C4 extends Object with M {}
|
||||
|
||||
void test(C1 c1, C2 c2, C3 c3, C4 c4) {
|
||||
if (c1._nonLate != null) {
|
||||
c1._nonLate.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c1._late != null) {
|
||||
c1._late.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c2._nonLate != null) {
|
||||
c2._nonLate.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c2._late != null) {
|
||||
c2._late.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c3._nonLate != null) {
|
||||
c3._nonLate.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c3._late != null) {
|
||||
c3._late.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c4._nonLate != null) {
|
||||
c4._nonLate.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
if (c4._late != null) {
|
||||
c4._late.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
}
|
||||
|
||||
main() {
|
||||
test(C1(), C2(), C3(), C4());
|
||||
}
|
Loading…
Reference in a new issue