Skip to content

Commit bb0580f

Browse files
scheglovCommit Queue
authored andcommitted
Elements. Unify interface-cycle handling for extension types
Unifies extension type cycle breaking with the existing `breakInterfaceCycles()` flow used for other elements and removes the bespoke walker in `summary2/extension_type.dart`. This consolidates the logic and ensures consistent behavior across element kinds. Key changes: - Move `hasImplementsSelfReference` and `hasRepresentationSelfReference` from fragments to `ExtensionTypeElementImpl`. Flags are now owned and serialized by the element. - Update summary I/O: writer emits and reader consumes the flags at the element level; fragment-level reads/writes are removed. - Adjust `ErrorVerifier` to read flags via `fragment.element`. - Handle extension types inside `interface_cycles.dart`: on an SCC, mark `hasImplementsSelfReference` and replace `interfaces` with `Object` or `Object?` depending on the representation’s nullability. - Bump `DATA_VERSION` to 524 due to summary layout changes. Why: - Eliminates duplicate, divergent cycle-breaking logic for extension types. - Clarifies ownership of self-reference state and simplifies serialization. - Produces consistent diagnostics and interface normalization across element kinds. Impact: - Requires a format bump; older summaries are invalidated as intended. - Fragment-level self-reference bits are removed in favor of element-level flags. Change-Id: Ifabb0acb2ce60353bbfe10ee5877c0c66cee5202 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445961 Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Paul Berry <[email protected]>
1 parent 37ac8e4 commit bb0580f

File tree

7 files changed

+30
-119
lines changed

7 files changed

+30
-119
lines changed

pkg/analyzer/lib/src/dart/analysis/driver.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ testFineAfterLibraryAnalyzerHook;
109109
// TODO(scheglov): Clean up the list of implicitly analyzed files.
110110
class AnalysisDriver {
111111
/// The version of data format, should be incremented on every format change.
112-
static const int DATA_VERSION = 523;
112+
static const int DATA_VERSION = 524;
113113

114114
/// The number of exception contexts allowed to write. Once this field is
115115
/// zero, we stop writing any new exception contexts in this process.

pkg/analyzer/lib/src/dart/element/element.dart

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2421,6 +2421,14 @@ class ExtensionTypeElementImpl extends InterfaceElementImpl
24212421
@override
24222422
final ExtensionTypeFragmentImpl _firstFragment;
24232423

2424+
/// Whether the element has direct or indirect reference to itself,
2425+
/// in representation.
2426+
bool hasRepresentationSelfReference = false;
2427+
2428+
/// Whether the element has direct or indirect reference to itself,
2429+
/// in implemented superinterfaces.
2430+
bool hasImplementsSelfReference = false;
2431+
24242432
late DartType _typeErasure;
24252433

24262434
ExtensionTypeElementImpl(this.reference, this._firstFragment) {
@@ -2443,30 +2451,6 @@ class ExtensionTypeElementImpl extends InterfaceElementImpl
24432451
];
24442452
}
24452453

2446-
/// Whether the element has direct or indirect reference to itself,
2447-
/// in implemented superinterfaces.
2448-
bool get hasImplementsSelfReference {
2449-
return _firstFragment.hasImplementsSelfReference;
2450-
}
2451-
2452-
/// Whether the element has direct or indirect reference to itself,
2453-
/// in implemented superinterfaces.
2454-
set hasImplementsSelfReference(bool value) {
2455-
_firstFragment.hasImplementsSelfReference = value;
2456-
}
2457-
2458-
/// Whether the element has direct or indirect reference to itself,
2459-
/// in representation.
2460-
bool get hasRepresentationSelfReference {
2461-
return _firstFragment.hasRepresentationSelfReference;
2462-
}
2463-
2464-
/// Whether the element has direct or indirect reference to itself,
2465-
/// in representation.
2466-
set hasRepresentationSelfReference(bool value) {
2467-
_firstFragment.hasRepresentationSelfReference = value;
2468-
}
2469-
24702454
@override
24712455
ElementKind get kind => ElementKind.EXTENSION_TYPE;
24722456

@@ -2535,14 +2519,6 @@ class ExtensionTypeFragmentImpl extends InterfaceFragmentImpl
25352519
@override
25362520
late final ExtensionTypeElementImpl element;
25372521

2538-
/// Whether the element has direct or indirect reference to itself,
2539-
/// in representation.
2540-
bool hasRepresentationSelfReference = false;
2541-
2542-
/// Whether the element has direct or indirect reference to itself,
2543-
/// in implemented superinterfaces.
2544-
bool hasImplementsSelfReference = false;
2545-
25462522
ExtensionTypeFragmentImpl({required super.name});
25472523

25482524
@override

pkg/analyzer/lib/src/generated/error_verifier.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3334,9 +3334,9 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
33343334

33353335
void _checkForExtensionTypeImplementsItself(
33363336
ExtensionTypeDeclarationImpl node,
3337-
ExtensionTypeFragmentImpl element,
3337+
ExtensionTypeFragmentImpl fragment,
33383338
) {
3339-
if (element.hasImplementsSelfReference) {
3339+
if (fragment.element.hasImplementsSelfReference) {
33403340
diagnosticReporter.atToken(
33413341
node.name,
33423342
CompileTimeErrorCode.extensionTypeImplementsItself,
@@ -3388,9 +3388,9 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
33883388

33893389
void _checkForExtensionTypeRepresentationDependsOnItself(
33903390
ExtensionTypeDeclarationImpl node,
3391-
ExtensionTypeFragmentImpl element,
3391+
ExtensionTypeFragmentImpl fragment,
33923392
) {
3393-
if (element.hasRepresentationSelfReference) {
3393+
if (fragment.element.hasRepresentationSelfReference) {
33943394
diagnosticReporter.atToken(
33953395
node.name,
33963396
CompileTimeErrorCode.extensionTypeRepresentationDependsOnItself,

pkg/analyzer/lib/src/summary2/bundle_reader.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,9 @@ class LibraryReader {
617617
var element = ExtensionTypeElementImpl(reference, fragments.first);
618618
element.linkFragments(fragments);
619619

620+
element.hasRepresentationSelfReference = _reader.readBool();
621+
element.hasImplementsSelfReference = _reader.readBool();
622+
620623
// TODO(scheglov): consider reading lazily
621624
for (var fragment in element.fragments) {
622625
fragment.ensureReadMembers();
@@ -648,8 +651,6 @@ class LibraryReader {
648651
create: (name) {
649652
var fragment = ExtensionTypeFragmentImpl(name: name);
650653
fragment.readModifiers(_reader);
651-
fragment.hasRepresentationSelfReference = _reader.readBool();
652-
fragment.hasImplementsSelfReference = _reader.readBool();
653654
fragment.typeParameters = _readTypeParameters();
654655

655656
// TODO(scheglov): consider reading lazily

pkg/analyzer/lib/src/summary2/bundle_writer.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,10 @@ class BundleWriter {
357357
_writeReference(element.reference);
358358
_writeFragments(element.fragments);
359359

360+
// TODO(fshcheglov): Put these separate flags into modifiers
361+
_sink.writeBool(element.hasRepresentationSelfReference);
362+
_sink.writeBool(element.hasImplementsSelfReference);
363+
360364
// TODO(scheglov): consider reading lazily
361365
_resolutionSink.withTypeParameters(element.typeParameters, () {
362366
_writeFieldElements(element.fields);
@@ -379,9 +383,6 @@ class BundleWriter {
379383

380384
void _writeExtensionTypeFragment(ExtensionTypeFragmentImpl fragment) {
381385
_writeTemplateFragment(fragment, () {
382-
// TODO(fshcheglov): Put these separate flags into modifiers
383-
_sink.writeBool(fragment.hasRepresentationSelfReference);
384-
_sink.writeBool(fragment.hasImplementsSelfReference);
385386
_writeTypeParameters(fragment.typeParameters, () {
386387
_resolutionSink._writeMetadata(fragment.metadata);
387388

pkg/analyzer/lib/src/summary2/extension_type.dart

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,6 @@ void buildExtensionTypes(Linker linker, List<AstNode> declarations) {
3030
for (var node in nodes) {
3131
walker.walk(node);
3232
}
33-
34-
_breakImplementsCycles(elements);
35-
}
36-
37-
/// Clears interfaces for extension types that have cycles.
38-
void _breakImplementsCycles(List<ExtensionTypeElementImpl> elements) {
39-
var walker = _ImplementsWalker();
40-
for (var element in elements) {
41-
var node = walker.getNode(element);
42-
walker.walk(node);
43-
}
4433
}
4534

4635
/// Collector of referenced extension types in a type.
@@ -60,63 +49,6 @@ class _DependenciesCollector extends RecursiveTypeVisitor {
6049
}
6150
}
6251

63-
class _ImplementsNode extends graph.Node<_ImplementsNode> {
64-
final _ImplementsWalker walker;
65-
final ExtensionTypeElementImpl element;
66-
67-
@override
68-
bool isEvaluated = false;
69-
70-
_ImplementsNode(this.walker, this.element);
71-
72-
@override
73-
List<_ImplementsNode> computeDependencies() {
74-
return element.interfaces
75-
.map((interface) => interface.element)
76-
.whereType<ExtensionTypeElementImpl>()
77-
.map(walker.getNode)
78-
.toList();
79-
}
80-
81-
void _evaluate() {
82-
isEvaluated = true;
83-
}
84-
85-
void _markCircular() {
86-
isEvaluated = true;
87-
element.hasImplementsSelfReference = true;
88-
89-
var representationType = element.representation.type;
90-
var typeSystem = element.library.typeSystem;
91-
92-
var superInterface =
93-
typeSystem.isNonNullable(representationType)
94-
? typeSystem.objectNone
95-
: typeSystem.objectQuestion;
96-
element.interfaces = [superInterface];
97-
}
98-
}
99-
100-
class _ImplementsWalker extends graph.DependencyWalker<_ImplementsNode> {
101-
final Map<ExtensionTypeElementImpl, _ImplementsNode> nodeMap = Map.identity();
102-
103-
@override
104-
void evaluate(_ImplementsNode v) {
105-
v._evaluate();
106-
}
107-
108-
@override
109-
void evaluateScc(List<_ImplementsNode> scc) {
110-
for (var node in scc) {
111-
node._markCircular();
112-
}
113-
}
114-
115-
_ImplementsNode getNode(ExtensionTypeElementImpl element) {
116-
return nodeMap[element] ??= _ImplementsNode(this, element);
117-
}
118-
}
119-
12052
class _Node extends graph.Node<_Node> {
12153
final _Walker walker;
12254
final ExtensionTypeDeclarationImpl node;

pkg/analyzer/lib/src/summary2/interface_cycles.dart

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,6 @@ void breakInterfaceCycles(Linker linker, List<AstNode> declarations) {
1414
for (var declaration in declarations) {
1515
if (declaration is DeclarationImpl) {
1616
var element = declaration.declaredFragment!.element;
17-
18-
// Handled elsewhere.
19-
if (element is ExtensionTypeElementImpl) {
20-
continue;
21-
}
22-
2317
if (element is InterfaceElementImpl) {
2418
elements.add(element);
2519
}
@@ -64,19 +58,26 @@ class _ImplementsNode extends graph.Node<_ImplementsNode> {
6458

6559
element.interfaceCycle = elements;
6660

61+
var typeProvider = element.library.typeProvider;
62+
var typeSystem = element.library.typeSystem;
63+
6764
switch (element) {
6865
case ClassElementImpl element:
69-
var typeProvider = element.library.typeProvider;
7066
element.supertype = typeProvider.objectType;
7167
element.mixins = [];
7268
element.interfaces = [];
7369
case EnumElementImpl element:
7470
element.mixins = [];
7571
element.interfaces = [];
7672
case ExtensionTypeElementImpl element:
77-
element.interfaces = [];
73+
element.hasImplementsSelfReference = true;
74+
var representationType = element.representation.type;
75+
var superInterface =
76+
typeSystem.isNonNullable(representationType)
77+
? typeSystem.objectNone
78+
: typeSystem.objectQuestion;
79+
element.interfaces = [superInterface];
7880
case MixinElementImpl element:
79-
var typeProvider = element.library.typeProvider;
8081
element.superclassConstraints = [typeProvider.objectType];
8182
element.interfaces = [];
8283
default:

0 commit comments

Comments
 (0)