Skip to content

Commit

Permalink
[cfe] Ensure that enum constructors are created during outline
Browse files Browse the repository at this point in the history
Enum constructors for enums with mixins were not built in the outline
phase because code assumes that initializers have to be present in order
to have a non-trivial constant constructor.

Closes #56681

Change-Id: Id614e3cde24ba3649db9ca67d938ba21575d7454
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384264
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
  • Loading branch information
johnniwinther authored and Commit Queue committed Sep 12, 2024
1 parent 3c1c13d commit e106b7b
Show file tree
Hide file tree
Showing 41 changed files with 757 additions and 151 deletions.
17 changes: 0 additions & 17 deletions pkg/front_end/lib/src/source/diet_listener.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import '../type_inference/type_inferrer.dart' show TypeInferrer;
import 'diet_parser.dart';
import 'offset_map.dart';
import 'source_constructor_builder.dart';
import 'source_enum_builder.dart';
import 'source_field_builder.dart';
import 'source_function_builder.dart';
import 'source_library_builder.dart' show SourceLibraryBuilder;
Expand Down Expand Up @@ -1180,22 +1179,6 @@ class DietListener extends StackListenerImpl {
int memberCount, Token endToken) {
debugEvent("Enum");
checkEmpty(enumKeyword.charOffset);

SourceEnumBuilder? enumBuilder = currentClass as SourceEnumBuilder?;
if (enumBuilder != null) {
DeclaredSourceConstructorBuilder? defaultConstructorBuilder =
enumBuilder.synthesizedDefaultConstructorBuilder;
if (defaultConstructorBuilder != null) {
BodyBuilder bodyBuilder = createFunctionListener(
defaultConstructorBuilder,
inOutlineBuildingPhase: false,
inMetadata: false,
inConstFields: false);
bodyBuilder.finishConstructor(AsyncMarker.Sync, new EmptyStatement(),
superParametersAsArguments: null);
}
}

currentDeclaration = null;
memberScope = libraryBuilder.scope;
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/front_end/lib/src/source/source_builder_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2005,6 +2005,8 @@ class BuilderFactoryImpl implements BuilderFactory, BuilderFactoryResult {
// into the outline. In case of super-parameters language feature, the
// super initializers are required to infer the types of super parameters.
constructorBuilder.beginInitializers =
// TODO(johnniwinther): Avoid using a dummy token to ensure building
// of constant constructors in the outline phase.
beginInitializers ?? new Token.eof(-1);
}
return constructorBuilder;
Expand Down
5 changes: 5 additions & 0 deletions pkg/front_end/lib/src/source/source_enum_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,11 @@ class SourceEnumBuilder extends SourceClassBuilder {
libraryName: libraryName),
forAbstractClassOrEnumOrMixin: true,
isSynthetic: true);
// Trick the constructor to be built during the outline phase.
// TODO(johnniwinther): Avoid relying on [beginInitializers] to ensure
// building constructors creation during the outline phase.
synthesizedDefaultConstructorBuilder!.beginInitializers =
new Token.eof(-1);
synthesizedDefaultConstructorBuilder!
.registerInitializedField(valuesBuilder);
nameSpace.addConstructor("", synthesizedDefaultConstructorBuilder!);
Expand Down
14 changes: 14 additions & 0 deletions pkg/front_end/testcases/dartdevc/issue56681/main.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) 2024, 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.

import 'main_lib.dart';

void main() {
const a = SomeEnum.value;
final b = SomeEnum.value;

print('a == b: ${a == b}');
print('a hash: ${a.hashCode}');
print('b hash: ${b.hashCode}');
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
library;
import self as self;
import "main_lib.dart" as mai;
import "dart:core" as core;

import "org-dartlang-testcase:///main_lib.dart";

static method main() → void {
const mai::SomeEnum a = #C3;
final mai::SomeEnum b = mai::SomeEnum::value;
core::print("a == b: ${a =={core::Object::==}{(core::Object) → core::bool} b}");
core::print("a hash: ${a.{core::Object::hashCode}{core::int}}");
core::print("b hash: ${b.{core::Object::hashCode}{core::int}}");
}

library;
import self as mai;
import "dart:core" as core;

abstract class HasSomeField extends core::Object /*isMixinDeclaration*/ {
abstract get someField() → core::String;
}
abstract class _SomeEnum&_Enum&HasSomeField = core::_Enum with mai::HasSomeField /*isAnonymousMixin,hasConstConstructor*/ {
const synthetic constructor •(core::int index, core::String _name) → mai::_SomeEnum&_Enum&HasSomeField
: super core::_Enum::•(index, _name)
;
abstract mixin-stub get someField() → core::String; -> mai::HasSomeField::someField
}
class SomeEnum extends mai::_SomeEnum&_Enum&HasSomeField /*isEnum*/ {
static const field core::List<mai::SomeEnum> values = #C4;
enum-element static const field mai::SomeEnum value = #C3;
const synthetic constructor •(core::int #index, core::String #name) → mai::SomeEnum
: super mai::_SomeEnum&_Enum&HasSomeField::•(#index, #name)
;
method _enumToString() → core::String
return "SomeEnum.${this.{core::_Enum::_name}{core::String}}";
@#C5
get someField() → core::String
return "field";
}

constants {
#C1 = 0.0
#C2 = "value"
#C3 = mai::SomeEnum {index:#C1, _name:#C2}
#C4 = <mai::SomeEnum>[#C3]
#C5 = core::_Override {}
}


Constructor coverage from constants:
org-dartlang-testcase:///main_lib.dart:
- SomeEnum. (from org-dartlang-testcase:///main_lib.dart:9:6)
- _SomeEnum&_Enum&HasSomeField. (from org-dartlang-testcase:///main_lib.dart:9:6)
- _Enum. (from org-dartlang-sdk:///lib/core/enum.dart)
- Object. (from org-dartlang-sdk:///lib/core/object.dart)
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
library;
import self as self;
import "main_lib.dart" as mai;
import "dart:core" as core;

import "org-dartlang-testcase:///main_lib.dart";

static method main() → void {
const mai::SomeEnum a = #C3;
final mai::SomeEnum b = mai::SomeEnum::value;
core::print("a == b: ${a =={core::Object::==}{(core::Object) → core::bool} b}");
core::print("a hash: ${a.{core::Object::hashCode}{core::int}}");
core::print("b hash: ${b.{core::Object::hashCode}{core::int}}");
}

constants {
#C1 = 0.0
#C2 = "value"
#C3 = mai::SomeEnum {index:#C1, _name:#C2}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
library;
import self as self;

import "org-dartlang-testcase:///main_lib.dart";

static method main() → void
;

library;
import self as self2;
import "dart:core" as core;

abstract class HasSomeField extends core::Object /*isMixinDeclaration*/ {
abstract get someField() → core::String;
}
abstract class _SomeEnum&_Enum&HasSomeField = core::_Enum with self2::HasSomeField /*isAnonymousMixin,hasConstConstructor*/ {
const synthetic constructor •(core::int index, core::String _name) → self2::_SomeEnum&_Enum&HasSomeField
: super core::_Enum::•(index, _name)
;
abstract mixin-stub get someField() → core::String; -> self2::HasSomeField::someField
}
class SomeEnum extends self2::_SomeEnum&_Enum&HasSomeField /*isEnum*/ {
static const field core::List<self2::SomeEnum> values = const <self2::SomeEnum>[self2::SomeEnum::value];
enum-element static const field self2::SomeEnum value = const self2::SomeEnum::•(0, "value");
const synthetic constructor •(core::int #index, core::String #name) → self2::SomeEnum
: super self2::_SomeEnum&_Enum&HasSomeField::•(#index, #name)
;
method _enumToString() → core::String
return "SomeEnum.${this.{core::_Enum::_name}{core::String}}";
@core::override
get someField() → core::String
;
}


Extra constant evaluation status:
Evaluated: StaticGet @ org-dartlang-testcase:///main_lib.dart:12:4 -> InstanceConstant(const _Override{})
Evaluated: ListLiteral @ org-dartlang-testcase:///main_lib.dart:9:6 -> ListConstant(const <SomeEnum>[const SomeEnum{_Enum.index: 0.0, _Enum._name: "value"}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///main_lib.dart:10:3 -> InstanceConstant(const SomeEnum{_Enum.index: 0.0, _Enum._name: "value"})
Extra constant evaluation: evaluated: 10, effectively constant: 3
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
library;
import self as self;
import "main_lib.dart" as mai;
import "dart:core" as core;

import "org-dartlang-testcase:///main_lib.dart";

static method main() → void {
const mai::SomeEnum a = #C3;
final mai::SomeEnum b = mai::SomeEnum::value;
core::print("a == b: ${a =={core::Object::==}{(core::Object) → core::bool} b}");
core::print("a hash: ${a.{core::Object::hashCode}{core::int}}");
core::print("b hash: ${b.{core::Object::hashCode}{core::int}}");
}

library;
import self as mai;
import "dart:core" as core;

abstract class HasSomeField extends core::Object /*isMixinDeclaration*/ {
abstract get someField() → core::String;
}
abstract class _SomeEnum&_Enum&HasSomeField = core::_Enum with mai::HasSomeField /*isAnonymousMixin,hasConstConstructor*/ {
const synthetic constructor •(core::int index, core::String _name) → mai::_SomeEnum&_Enum&HasSomeField
: super core::_Enum::•(index, _name)
;
abstract mixin-stub get someField() → core::String; -> mai::HasSomeField::someField
}
class SomeEnum extends mai::_SomeEnum&_Enum&HasSomeField /*isEnum*/ {
static const field core::List<mai::SomeEnum> values = #C4;
enum-element static const field mai::SomeEnum value = #C3;
const synthetic constructor •(core::int #index, core::String #name) → mai::SomeEnum
: super mai::_SomeEnum&_Enum&HasSomeField::•(#index, #name)
;
method _enumToString() → core::String
return "SomeEnum.${this.{core::_Enum::_name}{core::String}}";
@#C5
get someField() → core::String
return "field";
}

constants {
#C1 = 0.0
#C2 = "value"
#C3 = mai::SomeEnum {index:#C1, _name:#C2}
#C4 = <mai::SomeEnum>[#C3]
#C5 = core::_Override {}
}

Extra constant evaluation status:
Evaluated: StaticGet @ org-dartlang-testcase:///main.dart:9:22 -> InstanceConstant(const SomeEnum{_Enum.index: 0.0, _Enum._name: "value"})
Evaluated: VariableGetImpl @ org-dartlang-testcase:///main.dart:11:20 -> InstanceConstant(const SomeEnum{_Enum.index: 0.0, _Enum._name: "value"})
Evaluated: VariableGetImpl @ org-dartlang-testcase:///main.dart:12:20 -> InstanceConstant(const SomeEnum{_Enum.index: 0.0, _Enum._name: "value"})
Extra constant evaluation: evaluated: 21, effectively constant: 3


Constructor coverage from constants:
org-dartlang-testcase:///main_lib.dart:
- SomeEnum. (from org-dartlang-testcase:///main_lib.dart:9:6)
- _SomeEnum&_Enum&HasSomeField. (from org-dartlang-testcase:///main_lib.dart:9:6)
- _Enum. (from org-dartlang-sdk:///lib/core/enum.dart)
- Object. (from org-dartlang-sdk:///lib/core/object.dart)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import 'main_lib.dart';

void main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import 'main_lib.dart';

void main() {}
14 changes: 14 additions & 0 deletions pkg/front_end/testcases/dartdevc/issue56681/main_lib.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) 2024, 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.

mixin HasSomeField {
String get someField;
}

enum SomeEnum with HasSomeField {
value;

@override
String get someField => 'field';
}
1 change: 1 addition & 0 deletions pkg/front_end/testcases/dartdevc/issue56681/test.options
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
main_lib.dart
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ class E7 extends self::_E7&_Enum&MethodImplementation /*isEnum*/ {
static const field core::List<self::E7> values = const <self::E7>[self::E7::element];
enum-element static const field self::E7 element = const self::E7::•(0, "element");
const synthetic constructor •(core::int #index, core::String #name) → self::E7
: super self::_E7&_Enum&MethodImplementation::•(#index, #name)
;
method _enumToString() → core::String
return "E7.${this.{core::_Enum::_name}{core::String}}";
Expand All @@ -168,6 +169,6 @@ Evaluated: ListLiteral @ org-dartlang-testcase:///abstract_members.dart:35:6 ->
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///abstract_members.dart:36:3 -> InstanceConstant(const E5{_Enum.index: 0, _Enum._name: "element"})
Evaluated: ListLiteral @ org-dartlang-testcase:///abstract_members.dart:43:6 -> ListConstant(const <E6>[const E6{_Enum.index: 0, _Enum._name: "element"}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///abstract_members.dart:44:3 -> InstanceConstant(const E6{_Enum.index: 0, _Enum._name: "element"})
Evaluated: ListLiteral @ org-dartlang-testcase:///abstract_members.dart:51:6 -> ListConstant(const <E7>[const E7{}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///abstract_members.dart:52:3 -> InstanceConstant(const E7{})
Extra constant evaluation: evaluated: 50, effectively constant: 14
Evaluated: ListLiteral @ org-dartlang-testcase:///abstract_members.dart:51:6 -> ListConstant(const <E7>[const E7{_Enum.index: 0, _Enum._name: "element"}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///abstract_members.dart:52:3 -> InstanceConstant(const E7{_Enum.index: 0, _Enum._name: "element"})
Extra constant evaluation: evaluated: 52, effectively constant: 14
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ library;
// void element() {}
// ^^^^^^^
//
// pkg/front_end/testcases/enhanced_enums/conflicting_elements.dart:58:6: Error: A constant constructor can't call a non-constant super constructor.
// enum E9 with A9 {
// ^
//
import self as self;
import "dart:core" as core;

Expand Down Expand Up @@ -152,6 +156,7 @@ class E8 extends self::_E8&_Enum&A8 /*isEnum*/ {
static const field core::List<self::E8> values = const <self::E8>[self::E8::element];
enum-element static const field self::E8 element = const self::E8::•(0, "element");
const synthetic constructor •(core::int #index, core::String #name) → self::E8
: super self::_E8&_Enum&A8::•(#index, #name)
;
method _enumToString() → core::String
return "E8.${this.{core::_Enum::_name}{core::String}}";
Expand All @@ -172,6 +177,7 @@ class E9 extends self::_E9&_Enum&A9 /*isEnum*/ {
static const field core::List<self::E9> values = const <self::E9>[self::E9::element];
enum-element static const field self::E9 element = const self::E9::•(0, "element");
const synthetic constructor •(core::int #index, core::String #name) → self::E9
: super self::_E9&_Enum&A9::•(#index, #name)
;
method _enumToString() → core::String
return "E9.${this.{core::_Enum::_name}{core::String}}";
Expand All @@ -191,6 +197,7 @@ class E10 extends self::_E10&_Enum&A10 /*isEnum*/ {
static const field core::List<self::E10> values = const <self::E10>[self::E10::element];
enum-element static const field self::E10 element = const self::E10::•(0, "element");
const synthetic constructor •(core::int #index, core::String #name) → self::E10
: super self::_E10&_Enum&A10::•(#index, #name)
;
method _enumToString() → core::String
return "E10.${this.{core::_Enum::_name}{core::String}}";
Expand All @@ -210,10 +217,8 @@ Evaluated: ListLiteral @ org-dartlang-testcase:///conflicting_elements.dart:34:6
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///conflicting_elements.dart:35:3 -> InstanceConstant(const E6{_Enum.index: 0, _Enum._name: "element"})
Evaluated: ListLiteral @ org-dartlang-testcase:///conflicting_elements.dart:40:6 -> ListConstant(const <E7>[const E7{_Enum.index: 0, _Enum._name: "element"}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///conflicting_elements.dart:41:3 -> InstanceConstant(const E7{_Enum.index: 0, _Enum._name: "element"})
Evaluated: ListLiteral @ org-dartlang-testcase:///conflicting_elements.dart:50:6 -> ListConstant(const <E8>[const E8{}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///conflicting_elements.dart:51:3 -> InstanceConstant(const E8{})
Evaluated: ListLiteral @ org-dartlang-testcase:///conflicting_elements.dart:58:6 -> ListConstant(const <E9>[const E9{}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///conflicting_elements.dart:59:3 -> InstanceConstant(const E9{})
Evaluated: ListLiteral @ org-dartlang-testcase:///conflicting_elements.dart:66:6 -> ListConstant(const <E10>[const E10{}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///conflicting_elements.dart:67:3 -> InstanceConstant(const E10{})
Extra constant evaluation: evaluated: 72, effectively constant: 16
Evaluated: ListLiteral @ org-dartlang-testcase:///conflicting_elements.dart:50:6 -> ListConstant(const <E8>[const E8{_Enum.index: 0, _Enum._name: "element"}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///conflicting_elements.dart:51:3 -> InstanceConstant(const E8{_Enum.index: 0, _Enum._name: "element"})
Evaluated: ListLiteral @ org-dartlang-testcase:///conflicting_elements.dart:66:6 -> ListConstant(const <E10>[const E10{_Enum.index: 0, _Enum._name: "element"}])
Evaluated: ConstructorInvocation @ org-dartlang-testcase:///conflicting_elements.dart:67:3 -> InstanceConstant(const E10{_Enum.index: 0, _Enum._name: "element"})
Extra constant evaluation: evaluated: 79, effectively constant: 14
Loading

0 comments on commit e106b7b

Please sign in to comment.