Field type promotion: fix CFE handling of abstract fields.

Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: Ie35f7ff2d3de33c5f94bb7b703c13c9764a85c70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/269981
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This commit is contained in:
Paul Berry 2022-11-21 15:25:03 +00:00 committed by Commit Queue
parent a975bf0737
commit 22c46e573a
21 changed files with 593 additions and 9 deletions

View file

@ -1540,14 +1540,17 @@ class SourceClassBuilder extends ClassBuilderImpl
}
for (Procedure procedure in cls.procedures) {
// An instance getter makes fields with the same name unpromotable if it's
// concrete.
if (procedure.isGetter &&
procedure.isInstanceMember &&
!procedure.isAbstract &&
// concrete. Also, an abstract instance setter that's desugared from an
// abstract non-final field makes fields with the same name unpromotable.
if (procedure.isInstanceMember &&
_isPrivateNameInThisLibrary(procedure.name)) {
ProcedureStubKind procedureStubKind = procedure.stubKind;
if (procedureStubKind == ProcedureStubKind.Regular ||
procedureStubKind == ProcedureStubKind.NoSuchMethodForwarder) {
if (procedure.isGetter && !procedure.isAbstract) {
ProcedureStubKind procedureStubKind = procedure.stubKind;
if (procedureStubKind == ProcedureStubKind.Regular ||
procedureStubKind == ProcedureStubKind.NoSuchMethodForwarder) {
unpromotablePrivateFieldNames.add(procedure.name.text);
}
} else if (procedure.isSetter && procedure.isAbstractFieldAccessor) {
unpromotablePrivateFieldNames.add(procedure.name.text);
}
}

View file

@ -1612,6 +1612,7 @@ class AbstractOrExternalFieldEncoding implements FieldEncoding {
new FunctionNode(null, positionalParameters: [
new VariableDeclaration(extensionThisName)..fileOffset
]),
isAbstractFieldAccessor: isAbstract,
fileUri: fileUri,
reference: getterReference)
..fileOffset = charOffset
@ -1636,6 +1637,7 @@ class AbstractOrExternalFieldEncoding implements FieldEncoding {
returnType: const VoidType())
..fileOffset = charOffset
..fileEndOffset = charEndOffset,
isAbstractFieldAccessor: isAbstract,
fileUri: fileUri,
reference: setterReference)
..fileOffset = charOffset
@ -1648,7 +1650,9 @@ class AbstractOrExternalFieldEncoding implements FieldEncoding {
} else {
_getter = new Procedure(
dummyName, ProcedureKind.Getter, new FunctionNode(null),
fileUri: fileUri, reference: getterReference)
isAbstractFieldAccessor: isAbstract,
fileUri: fileUri,
reference: getterReference)
..fileOffset = charOffset
..fileEndOffset = charEndOffset
..isNonNullableByDefault = isNonNullableByDefault;
@ -1667,6 +1671,7 @@ class AbstractOrExternalFieldEncoding implements FieldEncoding {
positionalParameters: [parameter], returnType: const VoidType())
..fileOffset = charOffset
..fileEndOffset = charEndOffset,
isAbstractFieldAccessor: isAbstract,
fileUri: fileUri,
reference: setterReference)
..fileOffset = charOffset

View file

@ -347,7 +347,10 @@ class OperationsCfe
Set<String>? unpromotablePrivateFieldNames =
this.unpromotablePrivateFieldNames;
if (unpromotablePrivateFieldNames == null) return false;
if (property is! Field) return false;
if (property is Procedure && !property.isAbstractFieldAccessor) {
// We don't promote methods or explicit abstract getters.
return false;
}
String name = property.name.text;
if (!name.startsWith('_')) return false;
return !unpromotablePrivateFieldNames.contains(name);

View file

@ -0,0 +1,65 @@
// Copyright (c) 2022, 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 abstract fields.
// SharedOptions=--enable-experiment=inference-update-2
abstract class C {
abstract final int? _f1;
abstract int? _f2;
}
class D {
final int? _f1;
final int? _f2;
D(int i) : _f1 = i, _f2 = i;
}
void acceptsInt(int x) {}
void testAbstractFinalFieldIsPromotable(C c) {
if (c._f1 != null) {
var x = c._f1;
// `x` has type `int` so this is ok
acceptsInt(x);
}
}
void testAbstractNonFinalFieldIsNotPromotable(C c) {
// Technically, it would be sound to promote an abstract non-final field, but
// there's no point because it's just going to be implemented by a concrete
// non-final field or a getter/setter pair, which will prevent promotion. So
// we might as well prevent promotion even in the absence of an
// implementation.
if (c._f2 != null) {
var x = c._f2;
// `x` has type `int?` so this is ok
x = null;
}
}
void testAbstractFinalFieldDoesNotBlockPromotionElsewhere(D d) {
if (d._f1 != null) {
var x = d._f1;
// `x` has type `int` so this is ok
acceptsInt(x);
}
}
void testAbstractNonFinalFieldBlocksPromotionElsewhere(D d) {
// Technically, it would be sound if an abstract non-final field didn't block
// promotion, but there's no point because it's just going to be implemented
// by a concrete non-final field or a getter/setter pair, which will block
// promotion. So we might as well block promotion even in the absence of an
// implementation.
if (d._f2 != null) {
var x = d._f2;
// `x` has type `int?` so this is ok
x = null;
}
}
main() {}

View file

@ -0,0 +1,19 @@
abstract class C {
abstract final int? _f1;
abstract int? _f2;
}
class D {
final int? _f1;
final int? _f2;
D(int i)
: _f1 = i,
_f2 = i;
}
void acceptsInt(int x) {}
void testAbstractFinalFieldIsPromotable(C c) {}
void testAbstractNonFinalFieldIsNotPromotable(C c) {}
void testAbstractFinalFieldDoesNotBlockPromotionElsewhere(D d) {}
void testAbstractNonFinalFieldBlocksPromotionElsewhere(D d) {}
main() {}

View file

@ -0,0 +1,19 @@
abstract class C {
abstract final int? _f1;
abstract int? _f2;
}
class D {
D(int i)
: _f1 = i,
_f2 = i;
final int? _f1;
final int? _f2;
}
main() {}
void acceptsInt(int x) {}
void testAbstractFinalFieldDoesNotBlockPromotionElsewhere(D d) {}
void testAbstractFinalFieldIsPromotable(C c) {}
void testAbstractNonFinalFieldBlocksPromotionElsewhere(D d) {}
void testAbstractNonFinalFieldIsNotPromotable(C c) {}

View file

@ -0,0 +1,45 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
abstract class C extends core::Object {
synthetic constructor •() → self::C
: super core::Object::•()
;
abstract get _f1() → core::int?;
abstract get _f2() → core::int?;
abstract set _f2(core::int? #externalFieldValue) → void;
}
class D extends core::Object {
final field core::int? _f1;
final field core::int? _f2;
constructor •(core::int i) → self::D
: self::D::_f1 = i, self::D::_f2 = i, super core::Object::•()
;
}
static method acceptsInt(core::int x) → void {}
static method testAbstractFinalFieldIsPromotable(self::C c) → void {
if(!(c.{self::C::_f1}{core::int?} == null)) {
core::int x = c.{self::C::_f1}{core::int};
self::acceptsInt(x);
}
}
static method testAbstractNonFinalFieldIsNotPromotable(self::C c) → void {
if(!(c.{self::C::_f2}{core::int?} == null)) {
core::int? x = c.{self::C::_f2}{core::int?};
x = null;
}
}
static method testAbstractFinalFieldDoesNotBlockPromotionElsewhere(self::D d) → void {
if(!(d.{self::D::_f1}{core::int?} == null)) {
core::int x = d.{self::D::_f1}{core::int};
self::acceptsInt(x);
}
}
static method testAbstractNonFinalFieldBlocksPromotionElsewhere(self::D d) → void {
if(!(d.{self::D::_f2}{core::int?} == null)) {
core::int? x = d.{self::D::_f2}{core::int?};
x = null;
}
}
static method main() → dynamic {}

View file

@ -0,0 +1,45 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
abstract class C extends core::Object {
synthetic constructor •() → self::C
: super core::Object::•()
;
abstract get _f1() → core::int?;
abstract get _f2() → core::int?;
abstract set _f2(core::int? #externalFieldValue) → void;
}
class D extends core::Object {
final field core::int? _f1;
final field core::int? _f2;
constructor •(core::int i) → self::D
: self::D::_f1 = i, self::D::_f2 = i, super core::Object::•()
;
}
static method acceptsInt(core::int x) → void {}
static method testAbstractFinalFieldIsPromotable(self::C c) → void {
if(!(c.{self::C::_f1}{core::int?} == null)) {
core::int x = c.{self::C::_f1}{core::int};
self::acceptsInt(x);
}
}
static method testAbstractNonFinalFieldIsNotPromotable(self::C c) → void {
if(!(c.{self::C::_f2}{core::int?} == null)) {
core::int? x = c.{self::C::_f2}{core::int?};
x = null;
}
}
static method testAbstractFinalFieldDoesNotBlockPromotionElsewhere(self::D d) → void {
if(!(d.{self::D::_f1}{core::int?} == null)) {
core::int x = d.{self::D::_f1}{core::int};
self::acceptsInt(x);
}
}
static method testAbstractNonFinalFieldBlocksPromotionElsewhere(self::D d) → void {
if(!(d.{self::D::_f2}{core::int?} == null)) {
core::int? x = d.{self::D::_f2}{core::int?};
x = null;
}
}
static method main() → dynamic {}

View file

@ -0,0 +1,29 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
abstract class C extends core::Object {
synthetic constructor •() → self::C
;
abstract get _f1() → core::int?;
abstract get _f2() → core::int?;
abstract set _f2(core::int? #externalFieldValue) → void;
}
class D extends core::Object {
final field core::int? _f1;
final field core::int? _f2;
constructor •(core::int i) → self::D
;
}
static method acceptsInt(core::int x) → void
;
static method testAbstractFinalFieldIsPromotable(self::C c) → void
;
static method testAbstractNonFinalFieldIsNotPromotable(self::C c) → void
;
static method testAbstractFinalFieldDoesNotBlockPromotionElsewhere(self::D d) → void
;
static method testAbstractNonFinalFieldBlocksPromotionElsewhere(self::D d) → void
;
static method main() → dynamic
;

View file

@ -0,0 +1,45 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
abstract class C extends core::Object {
synthetic constructor •() → self::C
: super core::Object::•()
;
abstract get _f1() → core::int?;
abstract get _f2() → core::int?;
abstract set _f2(core::int? #externalFieldValue) → void;
}
class D extends core::Object {
final field core::int? _f1;
final field core::int? _f2;
constructor •(core::int i) → self::D
: self::D::_f1 = i, self::D::_f2 = i, super core::Object::•()
;
}
static method acceptsInt(core::int x) → void {}
static method testAbstractFinalFieldIsPromotable(self::C c) → void {
if(!(c.{self::C::_f1}{core::int?} == null)) {
core::int x = c.{self::C::_f1}{core::int};
self::acceptsInt(x);
}
}
static method testAbstractNonFinalFieldIsNotPromotable(self::C c) → void {
if(!(c.{self::C::_f2}{core::int?} == null)) {
core::int? x = c.{self::C::_f2}{core::int?};
x = null;
}
}
static method testAbstractFinalFieldDoesNotBlockPromotionElsewhere(self::D d) → void {
if(!(d.{self::D::_f1}{core::int?} == null)) {
core::int x = d.{self::D::_f1}{core::int};
self::acceptsInt(x);
}
}
static method testAbstractNonFinalFieldBlocksPromotionElsewhere(self::D d) → void {
if(!(d.{self::D::_f2}{core::int?} == null)) {
core::int? x = d.{self::D::_f2}{core::int?};
x = null;
}
}
static method main() → dynamic {}

View file

@ -0,0 +1,33 @@
// Copyright (c) 2022, 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 external fields. External
// fields are not promotable because they are effectively just getters; the
// language can't guarantee that they always return the same value.
abstract class C {
external final int? _f1;
}
extension on C {
external final int? _f2;
}
void testExternalFieldInClass(C c) {
if (c._f1 != null) {
var x = c._f1;
// `x` has type `int?` so this is ok
x = null;
}
}
void testExternalFieldInExtension(C c) {
if (c._f2 != null) {
var x = c._f2;
// `x` has type `int?` so this is ok
x = null;
}
}
main() {}

View file

@ -0,0 +1,11 @@
abstract class C {
external final int? _f1;
}
extension on C {
external final int? _f2;
}
void testExternalFieldInClass(C c) {}
void testExternalFieldInExtension(C c) {}
main() {}

View file

@ -0,0 +1,11 @@
abstract class C {
external final int? _f1;
}
extension on C {
external final int? _f2;
}
main() {}
void testExternalFieldInClass(C c) {}
void testExternalFieldInExtension(C c) {}

View file

@ -0,0 +1,27 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
abstract class C extends core::Object {
synthetic constructor •() → self::C
: super core::Object::•()
;
external get _f1() → core::int?;
}
extension /* unnamed */ _extension#0 on self::C {
get _f2 = self::_extension#0|get#_f2;
}
external static method _extension#0|get#_f2(self::C #this) → core::int?;
static method testExternalFieldInClass(self::C c) → void {
if(!(c.{self::C::_f1}{core::int?} == null)) {
core::int? x = c.{self::C::_f1}{core::int?};
x = null;
}
}
static method testExternalFieldInExtension(self::C c) → void {
if(!(self::_extension#0|get#_f2(c) == null)) {
core::int? x = self::_extension#0|get#_f2(c);
x = null;
}
}
static method main() → dynamic {}

View file

@ -0,0 +1,27 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
abstract class C extends core::Object {
synthetic constructor •() → self::C
: super core::Object::•()
;
external get _f1() → core::int?;
}
extension /* unnamed */ _extension#0 on self::C {
get _f2 = self::_extension#0|get#_f2;
}
external static method _extension#0|get#_f2(self::C #this) → core::int?;
static method testExternalFieldInClass(self::C c) → void {
if(!(c.{self::C::_f1}{core::int?} == null)) {
core::int? x = c.{self::C::_f1}{core::int?};
x = null;
}
}
static method testExternalFieldInExtension(self::C c) → void {
if(!(self::_extension#0|get#_f2(c) == null)) {
core::int? x = self::_extension#0|get#_f2(c);
x = null;
}
}
static method main() → dynamic {}

View file

@ -0,0 +1,19 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
abstract class C extends core::Object {
synthetic constructor •() → self::C
;
external get _f1() → core::int?;
}
extension /* unnamed */ _extension#0 on self::C {
get _f2 = self::_extension#0|get#_f2;
}
external static method _extension#0|get#_f2(self::C #this) → core::int?;
static method testExternalFieldInClass(self::C c) → void
;
static method testExternalFieldInExtension(self::C c) → void
;
static method main() → dynamic
;

View file

@ -0,0 +1,27 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
abstract class C extends core::Object {
synthetic constructor •() → self::C
: super core::Object::•()
;
external get _f1() → core::int?;
}
extension /* unnamed */ _extension#0 on self::C {
get _f2 = self::_extension#0|get#_f2;
}
external static method _extension#0|get#_f2(self::C #this) → core::int?;
static method testExternalFieldInClass(self::C c) → void {
if(!(c.{self::C::_f1}{core::int?} == null)) {
core::int? x = c.{self::C::_f1}{core::int?};
x = null;
}
}
static method testExternalFieldInExtension(self::C c) → void {
if(!(self::_extension#0|get#_f2(c) == null)) {
core::int? x = self::_extension#0|get#_f2(c);
x = null;
}
}
static method main() → dynamic {}

View file

@ -3215,6 +3215,7 @@ class Procedure extends Member {
bool isConst = false,
bool isExtensionMember = false,
bool isSynthetic = false,
bool isAbstractFieldAccessor = false,
int transformerFlags = 0,
required Uri fileUri,
Reference? reference,
@ -3227,6 +3228,7 @@ class Procedure extends Member {
isConst: isConst,
isExtensionMember: isExtensionMember,
isSynthetic: isSynthetic,
isAbstractFieldAccessor: isAbstractFieldAccessor,
transformerFlags: transformerFlags,
fileUri: fileUri,
reference: reference,
@ -3241,6 +3243,7 @@ class Procedure extends Member {
bool isConst = false,
bool isExtensionMember = false,
bool isSynthetic = false,
bool isAbstractFieldAccessor = false,
int transformerFlags = 0,
required Uri fileUri,
Reference? reference,
@ -3258,6 +3261,7 @@ class Procedure extends Member {
this.isConst = isConst;
this.isExtensionMember = isExtensionMember;
this.isSynthetic = isSynthetic;
this.isAbstractFieldAccessor = isAbstractFieldAccessor;
setTransformerFlagsWithoutLazyLoading(transformerFlags);
assert(!(isMemberSignature && stubTargetReference == null),
"No member signature origin for member signature $this.");
@ -3308,6 +3312,7 @@ class Procedure extends Member {
static const int FlagNonNullableByDefault = 1 << 6;
static const int FlagSynthetic = 1 << 7;
static const int FlagInternalImplementation = 1 << 8;
static const int FlagIsAbstractFieldAccessor = 1 << 9;
bool get isStatic => flags & FlagStatic != 0;
@ -3375,6 +3380,15 @@ class Procedure extends Member {
: (flags & ~FlagInternalImplementation);
}
/// If `true` this procedure was generated from an abstract field.
bool get isAbstractFieldAccessor => flags & FlagIsAbstractFieldAccessor != 0;
void set isAbstractFieldAccessor(bool value) {
flags = value
? (flags | FlagIsAbstractFieldAccessor)
: (flags & ~FlagIsAbstractFieldAccessor);
}
@override
bool get isExtensionMember => flags & FlagExtensionMember != 0;

View file

@ -0,0 +1,67 @@
// Copyright (c) 2022, 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 abstract fields.
// SharedOptions=--enable-experiment=inference-update-2
abstract class C {
abstract final int? _f1;
abstract int? _f2;
}
class D {
final int? _f1;
final int? _f2;
D(int i)
: _f1 = i,
_f2 = i;
}
void acceptsInt(int x) {}
void testAbstractFinalFieldIsPromotable(C c) {
if (c._f1 != null) {
var x = c._f1;
// `x` has type `int` so this is ok
acceptsInt(x);
}
}
void testAbstractNonFinalFieldIsNotPromotable(C c) {
// Technically, it would be sound to promote an abstract non-final field, but
// there's no point because it's just going to be implemented by a concrete
// non-final field or a getter/setter pair, which will prevent promotion. So
// we might as well prevent promotion even in the absence of an
// implementation.
if (c._f2 != null) {
var x = c._f2;
// `x` has type `int?` so this is ok
x = null;
}
}
void testAbstractFinalFieldDoesNotBlockPromotionElsewhere(D d) {
if (d._f1 != null) {
var x = d._f1;
// `x` has type `int` so this is ok
acceptsInt(x);
}
}
void testAbstractNonFinalFieldBlocksPromotionElsewhere(D d) {
// Technically, it would be sound if an abstract non-final field didn't block
// promotion, but there's no point because it's just going to be implemented
// by a concrete non-final field or a getter/setter pair, which will block
// promotion. So we might as well block promotion even in the absence of an
// implementation.
if (d._f2 != null) {
var x = d._f2;
// `x` has type `int?` so this is ok
x = null;
}
}
main() {}

View file

@ -0,0 +1,42 @@
// Copyright (c) 2022, 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 does not try to promote abstract getters.
// SharedOptions=--enable-experiment=inference-update-2
abstract class C {
int? get _f;
}
class D extends C {
final int? _f;
D(this._f);
}
void acceptsInt(int x) {}
void testBaseClass(C c) {
if (c._f != null) {
var x = c._f;
// `x` has type `int?` so this is ok
x = null;
}
}
void testDerivedClass(D d) {
if (d._f != null) {
var x = d._f;
// `x` has type `int` so this is ok
acceptsInt(x);
}
}
main() {
for (var f in [null, 0]) {
testBaseClass(D(f));
testDerivedClass(D(f));
}
}

View file

@ -0,0 +1,28 @@
// Copyright (c) 2022, 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 does not try to promote method tearoffs.
// SharedOptions=--enable-experiment=inference-update-2
import '../static_type_helper.dart';
class C {
num _f(int i) => 0;
}
class D extends C {
int _f(num i) => 0;
}
void test(C c) {
if (c._f is int Function(num)) {
var x = c._f;
x.expectStaticType<Exactly<num Function(int)>>();
}
}
main() {
test(D());
}