Fine. Persist isSimplyBounded; track isValidMixin indirectly.
Capture the simply-boundedness of instance elements in the fine-grained manifest and stop baking `isValidMixin` into item IDs. What: - Add `isSimplyBounded` to `InstanceItem` and all subclasses (`ClassItem`, `EnumItem`, `ExtensionItem`, `ExtensionTypeItem`, `MixinItem`). Persist it in summaries and compare it in `match`. - Update the summary schema to write/read `isSimplyBounded` immediately after `typeParameters` for all instance items. - Reclassify `ClassElementImpl.isValidMixin` from `@trackedIncludedInId` to `@trackedIndirectly`. - Bump `AnalysisDriver.DATA_VERSION` from 533 to 536. Why: - `isSimplyBounded` is an observable property of generic declarations. Not recording it could allow identity reuse when simply-boundedness changes without other shape differences. Persisting it yields more precise matching and invalidation. - `isValidMixin` is derived from other fields (e.g., `supertype`, constructors). Tracking it indirectly avoids unnecessary ID churn and reduces over-invalidation while remaining responsive to the inputs it depends on. Impact: - More accurate fine-grained dependency tracking for generics. - Fewer spurious item-ID changes from mixin validity checks. - Existing caches are invalidated due to the data format bump; no public API changes. Change-Id: I2700b16add664a4fd127f492fb186c8e1f3635e9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448100 Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com> Reviewed-by: Paul Berry <paulberry@google.com>
This commit is contained in:
committed by
Commit Queue
parent
579ad244df
commit
3cc5f5669b
@@ -106,7 +106,7 @@ testFineAfterLibraryAnalyzerHook;
|
||||
// TODO(scheglov): Clean up the list of implicitly analyzed files.
|
||||
class AnalysisDriver {
|
||||
/// The version of data format, should be incremented on every format change.
|
||||
static const int DATA_VERSION = 534;
|
||||
static const int DATA_VERSION = 536;
|
||||
|
||||
/// The number of exception contexts allowed to write. Once this field is
|
||||
/// zero, we stop writing any new exception contexts in this process.
|
||||
|
||||
@@ -338,7 +338,7 @@ class ClassElementImpl extends InterfaceElementImpl implements ClassElement {
|
||||
}
|
||||
|
||||
@override
|
||||
@trackedIncludedInId
|
||||
@trackedIndirectly
|
||||
bool get isValidMixin {
|
||||
var supertype = this.supertype;
|
||||
if (supertype != null && !supertype.isDartCoreObject) {
|
||||
|
||||
@@ -28,6 +28,7 @@ class ClassItem extends InterfaceItem<ClassElementImpl> {
|
||||
required super.id,
|
||||
required super.metadata,
|
||||
required super.typeParameters,
|
||||
required super.isSimplyBounded,
|
||||
required super.declaredConflicts,
|
||||
required super.declaredFields,
|
||||
required super.declaredGetters,
|
||||
@@ -58,6 +59,7 @@ class ClassItem extends InterfaceItem<ClassElementImpl> {
|
||||
id: id,
|
||||
metadata: ManifestMetadata.encode(context, element.metadata),
|
||||
typeParameters: typeParameters,
|
||||
isSimplyBounded: element.isSimplyBounded,
|
||||
declaredConflicts: {},
|
||||
declaredFields: {},
|
||||
declaredGetters: {},
|
||||
@@ -85,6 +87,7 @@ class ClassItem extends InterfaceItem<ClassElementImpl> {
|
||||
id: ManifestItemId.read(reader),
|
||||
metadata: ManifestMetadata.read(reader),
|
||||
typeParameters: ManifestTypeParameter.readList(reader),
|
||||
isSimplyBounded: reader.readBool(),
|
||||
declaredConflicts: reader.readLookupNameToIdMap(),
|
||||
declaredFields: InstanceItemFieldItem.readMap(reader),
|
||||
declaredGetters: InstanceItemGetterItem.readMap(reader),
|
||||
@@ -136,6 +139,7 @@ class EnumItem extends InterfaceItem<EnumElementImpl> {
|
||||
required super.id,
|
||||
required super.metadata,
|
||||
required super.typeParameters,
|
||||
required super.isSimplyBounded,
|
||||
required super.declaredConflicts,
|
||||
required super.declaredFields,
|
||||
required super.declaredGetters,
|
||||
@@ -159,6 +163,7 @@ class EnumItem extends InterfaceItem<EnumElementImpl> {
|
||||
id: id,
|
||||
metadata: ManifestMetadata.encode(context, element.metadata),
|
||||
typeParameters: typeParameters,
|
||||
isSimplyBounded: element.isSimplyBounded,
|
||||
declaredConflicts: {},
|
||||
declaredFields: {},
|
||||
declaredGetters: {},
|
||||
@@ -179,6 +184,7 @@ class EnumItem extends InterfaceItem<EnumElementImpl> {
|
||||
id: ManifestItemId.read(reader),
|
||||
metadata: ManifestMetadata.read(reader),
|
||||
typeParameters: ManifestTypeParameter.readList(reader),
|
||||
isSimplyBounded: reader.readBool(),
|
||||
declaredConflicts: reader.readLookupNameToIdMap(),
|
||||
declaredFields: InstanceItemFieldItem.readMap(reader),
|
||||
declaredGetters: InstanceItemGetterItem.readMap(reader),
|
||||
@@ -202,6 +208,7 @@ class ExtensionItem<E extends ExtensionElementImpl> extends InstanceItem<E> {
|
||||
required super.id,
|
||||
required super.metadata,
|
||||
required super.typeParameters,
|
||||
required super.isSimplyBounded,
|
||||
required super.declaredConflicts,
|
||||
required super.declaredFields,
|
||||
required super.declaredGetters,
|
||||
@@ -222,6 +229,7 @@ class ExtensionItem<E extends ExtensionElementImpl> extends InstanceItem<E> {
|
||||
id: id,
|
||||
metadata: ManifestMetadata.encode(context, element.metadata),
|
||||
typeParameters: typeParameters,
|
||||
isSimplyBounded: element.isSimplyBounded,
|
||||
declaredConflicts: {},
|
||||
declaredFields: {},
|
||||
declaredGetters: {},
|
||||
@@ -239,6 +247,7 @@ class ExtensionItem<E extends ExtensionElementImpl> extends InstanceItem<E> {
|
||||
id: ManifestItemId.read(reader),
|
||||
metadata: ManifestMetadata.read(reader),
|
||||
typeParameters: ManifestTypeParameter.readList(reader),
|
||||
isSimplyBounded: reader.readBool(),
|
||||
declaredConflicts: reader.readLookupNameToIdMap(),
|
||||
declaredFields: InstanceItemFieldItem.readMap(reader),
|
||||
declaredGetters: InstanceItemGetterItem.readMap(reader),
|
||||
@@ -273,6 +282,7 @@ class ExtensionTypeItem extends InterfaceItem<ExtensionTypeElementImpl> {
|
||||
required super.id,
|
||||
required super.metadata,
|
||||
required super.typeParameters,
|
||||
required super.isSimplyBounded,
|
||||
required super.declaredConflicts,
|
||||
required super.declaredFields,
|
||||
required super.declaredGetters,
|
||||
@@ -300,6 +310,7 @@ class ExtensionTypeItem extends InterfaceItem<ExtensionTypeElementImpl> {
|
||||
id: id,
|
||||
metadata: ManifestMetadata.encode(context, element.metadata),
|
||||
typeParameters: typeParameters,
|
||||
isSimplyBounded: element.isSimplyBounded,
|
||||
declaredConflicts: {},
|
||||
declaredFields: {},
|
||||
declaredGetters: {},
|
||||
@@ -324,6 +335,7 @@ class ExtensionTypeItem extends InterfaceItem<ExtensionTypeElementImpl> {
|
||||
id: ManifestItemId.read(reader),
|
||||
metadata: ManifestMetadata.read(reader),
|
||||
typeParameters: ManifestTypeParameter.readList(reader),
|
||||
isSimplyBounded: reader.readBool(),
|
||||
declaredConflicts: reader.readLookupNameToIdMap(),
|
||||
declaredFields: InstanceItemFieldItem.readMap(reader),
|
||||
declaredGetters: InstanceItemGetterItem.readMap(reader),
|
||||
@@ -366,6 +378,7 @@ class ExtensionTypeItem extends InterfaceItem<ExtensionTypeElementImpl> {
|
||||
sealed class InstanceItem<E extends InstanceElementImpl>
|
||||
extends TopLevelItem<E> {
|
||||
final List<ManifestTypeParameter> typeParameters;
|
||||
final bool isSimplyBounded;
|
||||
|
||||
/// The names of duplicate or otherwise conflicting members.
|
||||
/// Such names will not be added to `declaredXyz` maps.
|
||||
@@ -382,6 +395,7 @@ sealed class InstanceItem<E extends InstanceElementImpl>
|
||||
required super.id,
|
||||
required super.metadata,
|
||||
required this.typeParameters,
|
||||
required this.isSimplyBounded,
|
||||
required this.declaredConflicts,
|
||||
required this.declaredFields,
|
||||
required this.declaredGetters,
|
||||
@@ -550,13 +564,15 @@ sealed class InstanceItem<E extends InstanceElementImpl>
|
||||
bool match(MatchContext context, E element) {
|
||||
context.addTypeParameters(element.typeParameters);
|
||||
return super.match(context, element) &&
|
||||
typeParameters.match(context, element.typeParameters);
|
||||
typeParameters.match(context, element.typeParameters) &&
|
||||
isSimplyBounded == element.isSimplyBounded;
|
||||
}
|
||||
|
||||
@override
|
||||
void write(BufferedSink sink) {
|
||||
super.write(sink);
|
||||
typeParameters.write(sink);
|
||||
sink.writeBool(isSimplyBounded);
|
||||
declaredConflicts.write(sink);
|
||||
declaredFields.write(sink);
|
||||
declaredGetters.write(sink);
|
||||
@@ -918,6 +934,7 @@ sealed class InterfaceItem<E extends InterfaceElementImpl>
|
||||
required super.id,
|
||||
required super.metadata,
|
||||
required super.typeParameters,
|
||||
required super.isSimplyBounded,
|
||||
required super.declaredConflicts,
|
||||
required super.declaredFields,
|
||||
required super.declaredGetters,
|
||||
@@ -1220,6 +1237,7 @@ class MixinItem extends InterfaceItem<MixinElementImpl> {
|
||||
required super.id,
|
||||
required super.metadata,
|
||||
required super.typeParameters,
|
||||
required super.isSimplyBounded,
|
||||
required super.supertype,
|
||||
required super.interfaces,
|
||||
required super.mixins,
|
||||
@@ -1248,6 +1266,7 @@ class MixinItem extends InterfaceItem<MixinElementImpl> {
|
||||
id: id,
|
||||
metadata: ManifestMetadata.encode(context, element.metadata),
|
||||
typeParameters: typeParameters,
|
||||
isSimplyBounded: element.isSimplyBounded,
|
||||
declaredConflicts: {},
|
||||
declaredFields: {},
|
||||
declaredGetters: {},
|
||||
@@ -1273,6 +1292,7 @@ class MixinItem extends InterfaceItem<MixinElementImpl> {
|
||||
id: ManifestItemId.read(reader),
|
||||
metadata: ManifestMetadata.read(reader),
|
||||
typeParameters: ManifestTypeParameter.readList(reader),
|
||||
isSimplyBounded: reader.readBool(),
|
||||
declaredConflicts: reader.readLookupNameToIdMap(),
|
||||
declaredFields: InstanceItemFieldItem.readMap(reader),
|
||||
declaredGetters: InstanceItemGetterItem.readMap(reader),
|
||||
|
||||
@@ -12278,6 +12278,105 @@ class A {
|
||||
);
|
||||
}
|
||||
|
||||
test_dependency_class_namedConstructor_add_isValidMixin() async {
|
||||
configuration.withStreamResolvedUnitResults = false;
|
||||
|
||||
_ManualRequirements.install((state) {
|
||||
state.singleUnit.scopeClassElement('A').isValidMixin;
|
||||
});
|
||||
|
||||
await _runChangeScenarioTA(
|
||||
initialA: r'''
|
||||
class A {}
|
||||
''',
|
||||
testCode: r'''
|
||||
import 'a.dart';
|
||||
''',
|
||||
operation: _FineOperationTestFileGetErrors(),
|
||||
expectedInitialEvents: r'''
|
||||
[status] working
|
||||
[operation] linkLibraryCycle SDK
|
||||
[operation] linkLibraryCycle
|
||||
package:test/a.dart
|
||||
declaredClasses
|
||||
A: #M0
|
||||
interface: #M1
|
||||
requirements
|
||||
[operation] linkLibraryCycle
|
||||
package:test/test.dart
|
||||
requirements
|
||||
[operation] analyzeFile
|
||||
file: /home/test/lib/test.dart
|
||||
library: /home/test/lib/test.dart
|
||||
[operation] analyzedLibrary
|
||||
file: /home/test/lib/test.dart
|
||||
requirements
|
||||
libraries
|
||||
package:test/a.dart
|
||||
exportedTopLevels
|
||||
A: #M0
|
||||
interfaces
|
||||
A
|
||||
allConstructors: #M2
|
||||
[status] idle
|
||||
[future] getErrors T1
|
||||
ErrorsResult #0
|
||||
path: /home/test/lib/test.dart
|
||||
uri: package:test/test.dart
|
||||
flags: isLibrary
|
||||
errors
|
||||
7 +8 UNUSED_IMPORT
|
||||
''',
|
||||
updatedA: r'''
|
||||
class A {
|
||||
A.named();
|
||||
}
|
||||
''',
|
||||
expectedUpdatedEvents: r'''
|
||||
[status] working
|
||||
[operation] linkLibraryCycle
|
||||
package:test/a.dart
|
||||
declaredClasses
|
||||
A: #M0
|
||||
declaredConstructors
|
||||
named: #M3
|
||||
interface: #M1
|
||||
requirements
|
||||
[operation] reuseLinkedBundle
|
||||
package:test/test.dart
|
||||
[operation] checkLibraryDiagnosticsRequirements
|
||||
library: /home/test/lib/test.dart
|
||||
interfaceChildrenIdsMismatch
|
||||
libraryUri: package:test/a.dart
|
||||
interfaceName: A
|
||||
childrenPropertyName: constructors
|
||||
expectedIds: #M2
|
||||
actualIds: #M3
|
||||
[operation] analyzeFile
|
||||
file: /home/test/lib/test.dart
|
||||
library: /home/test/lib/test.dart
|
||||
[operation] analyzedLibrary
|
||||
file: /home/test/lib/test.dart
|
||||
requirements
|
||||
libraries
|
||||
package:test/a.dart
|
||||
exportedTopLevels
|
||||
A: #M0
|
||||
interfaces
|
||||
A
|
||||
allConstructors: #M3
|
||||
[status] idle
|
||||
[future] getErrors T2
|
||||
ErrorsResult #1
|
||||
path: /home/test/lib/test.dart
|
||||
uri: package:test/test.dart
|
||||
flags: isLibrary
|
||||
errors
|
||||
7 +8 UNUSED_IMPORT
|
||||
''',
|
||||
);
|
||||
}
|
||||
|
||||
test_dependency_class_namedConstructor_change_getConstructors() async {
|
||||
configuration.includeDefaultConstructors();
|
||||
|
||||
@@ -44680,6 +44779,38 @@ class D {}
|
||||
);
|
||||
}
|
||||
|
||||
test_manifest_class_modifier_isSimplyBounded() async {
|
||||
await _runLibraryManifestScenario(
|
||||
initialCode: r'''
|
||||
class A<T> {}
|
||||
class B<T extends List<T>> {}
|
||||
''',
|
||||
expectedInitialEvents: r'''
|
||||
[operation] linkLibraryCycle SDK
|
||||
[operation] linkLibraryCycle
|
||||
package:test/test.dart
|
||||
declaredClasses
|
||||
A: #M0
|
||||
interface: #M1
|
||||
B: #M2
|
||||
interface: #M3
|
||||
''',
|
||||
updatedCode: r'''
|
||||
class A<T extends List<T>> {}
|
||||
class B<T> {}
|
||||
''',
|
||||
expectedUpdatedEvents: r'''
|
||||
[operation] linkLibraryCycle
|
||||
package:test/test.dart
|
||||
declaredClasses
|
||||
A: #M4
|
||||
interface: #M5
|
||||
B: #M6
|
||||
interface: #M7
|
||||
''',
|
||||
);
|
||||
}
|
||||
|
||||
test_manifest_class_private() async {
|
||||
await _runLibraryManifestScenario(
|
||||
initialCode: r'''
|
||||
|
||||
Reference in New Issue
Block a user