Skip to content

Commit b54e38e

Browse files
scheglovCommit Queue
authored andcommitted
Fine. Record and match type-parameter variance in manifests.
Capture declaration-site variance for generic declarations in the fine manifests and use it during matching. Previously, manifests ignored variance, so declarations that differed only by variance could be incorrectly considered equivalent (or vice versa). Including variance closes that correctness gap and prevents unstable reuse decisions. Key changes: - Encode variance on each ManifestTypeParameter (using shared.Variance) and persist it in the wire format (written before the bound). - Compare variance when matching type parameters, affecting classes, mixins, function types, and type aliases. - Consider type parameters when matching TypeAliasItem to avoid false-positive matches of otherwise identical aliases. - Switch withTypeParameters and match sites to work with TypeParameterElementImpl so variance is available everywhere. - Update the result printer to display type-parameter variance with ordinal indices. - Bump AnalysisDriver.DATA_VERSION to 531 to invalidate stale caches. Notes: - Default (implicit) covariant parameters and explicit `out` parameters are treated equivalently, ensuring stable IDs in that case. - This is an internal format change; no public API surface is altered. Rationale: Storing and matching variance makes manifest-based reuse sensitive to a semantically observable part of generic APIs. This yields more accurate incremental analysis decisions and avoids reusing results across variance-incompatible declarations. Change-Id: I397016773c2bae8b7e588dd2b5cbdee303bbfb91 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447945 Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 740e819 commit b54e38e

File tree

6 files changed

+227
-20
lines changed

6 files changed

+227
-20
lines changed

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

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

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

pkg/analyzer/lib/src/fine/manifest_context.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
import 'package:analyzer/dart/element/element.dart';
6+
import 'package:analyzer/src/dart/element/element.dart';
67
import 'package:analyzer/src/fine/lookup_name.dart';
78
import 'package:analyzer/src/fine/manifest_id.dart';
89
import 'package:analyzer/src/fine/manifest_item.dart';
@@ -56,7 +57,7 @@ class EncodeContext {
5657
}
5758

5859
T withTypeParameters<T>(
59-
List<TypeParameterElement> typeParameters,
60+
List<TypeParameterElementImpl> typeParameters,
6061
T Function(List<ManifestTypeParameter> typeParameters) operation,
6162
) {
6263
for (var typeParameter in typeParameters) {

pkg/analyzer/lib/src/fine/manifest_item.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,6 +1498,7 @@ class TypeAliasItem extends TopLevelItem<TypeAliasElementImpl> {
14981498
bool match(MatchContext context, TypeAliasElementImpl element) {
14991499
context.addTypeParameters(element.typeParameters);
15001500
return super.match(context, element) &&
1501+
typeParameters.match(context, element.typeParameters) &&
15011502
aliasedType.match(context, element.aliasedType);
15021503
}
15031504

pkg/analyzer/lib/src/fine/manifest_type.dart

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'package:analyzer/dart/element/element.dart';
5+
import 'package:_fe_analyzer_shared/src/types/shared_type.dart'
6+
as shared
7+
show Variance;
68
import 'package:analyzer/dart/element/nullability_suffix.dart';
79
import 'package:analyzer/dart/element/type.dart';
810
import 'package:analyzer/src/dart/element/element.dart';
@@ -539,26 +541,34 @@ sealed class ManifestType {
539541
}
540542

541543
class ManifestTypeParameter {
544+
final shared.Variance variance;
542545
final ManifestType? bound;
543546

544547
factory ManifestTypeParameter.encode(
545548
EncodeContext context,
546-
TypeParameterElement element,
549+
TypeParameterElementImpl element,
547550
) {
548-
return ManifestTypeParameter._(bound: element.bound?.encode(context));
551+
return ManifestTypeParameter._(
552+
variance: element.variance,
553+
bound: element.bound?.encode(context),
554+
);
549555
}
550556

551557
factory ManifestTypeParameter.read(SummaryDataReader reader) {
552-
return ManifestTypeParameter._(bound: ManifestType.readOptional(reader));
558+
return ManifestTypeParameter._(
559+
variance: reader.readEnum(shared.Variance.values),
560+
bound: ManifestType.readOptional(reader),
561+
);
553562
}
554563

555-
ManifestTypeParameter._({required this.bound});
564+
ManifestTypeParameter._({required this.variance, required this.bound});
556565

557-
bool match(MatchContext context, TypeParameterElement element) {
558-
return bound.match(context, element.bound);
566+
bool match(MatchContext context, TypeParameterElementImpl element) {
567+
return element.variance == variance && bound.match(context, element.bound);
559568
}
560569

561570
void write(BufferedSink sink) {
571+
sink.writeEnum(variance);
562572
bound.writeOptional(sink);
563573
}
564574

@@ -702,7 +712,7 @@ extension ListOfManifestTypeExtension on List<ManifestType> {
702712
}
703713

704714
extension ListOfManifestTypeParameterExtension on List<ManifestTypeParameter> {
705-
bool match(MatchContext context, List<TypeParameterElement> elements) {
715+
bool match(MatchContext context, List<TypeParameterElementImpl> elements) {
706716
if (elements.length != length) {
707717
return false;
708718
}

pkg/analyzer/test/src/dart/analysis/driver_test.dart

Lines changed: 195 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44150,13 +44150,15 @@ class A<T> {
4415044150
declaredClasses
4415144151
A: #M0
4415244152
typeParameters
44153-
bound: <null>
44153+
#0 covariant
44154+
bound: <null>
4415444155
supertype: Object @ dart:core
4415544156
declaredMethods
4415644157
foo: #M1
4415744158
functionType: FunctionType
4415844159
typeParameters
44159-
bound: <null>
44160+
#0 covariant
44161+
bound: <null>
4416044162
returnType: Map @ dart:core
4416144163
typeParameter#1
4416244164
typeParameter#0
@@ -44176,7 +44178,8 @@ class A<T> {
4417644178
declaredClasses
4417744179
A: #M0
4417844180
typeParameters
44179-
bound: <null>
44181+
#0 covariant
44182+
bound: <null>
4418044183
supertype: Object @ dart:core
4418144184
declaredMethods
4418244185
bar: #M3
@@ -44185,7 +44188,8 @@ class A<T> {
4418544188
foo: #M1
4418644189
functionType: FunctionType
4418744190
typeParameters
44188-
bound: <null>
44191+
#0 covariant
44192+
bound: <null>
4418944193
returnType: Map @ dart:core
4419044194
typeParameter#1
4419144195
typeParameter#0
@@ -46008,6 +46012,82 @@ class A<T> {}
4600846012
);
4600946013
}
4601046014

46015+
test_manifest_class_typeParameters_variance() async {
46016+
await _runLibraryManifestScenario(
46017+
initialCode: r'''
46018+
class A<in T> {}
46019+
class B<out T> {}
46020+
class C<in T> {}
46021+
class D<out T> {}
46022+
''',
46023+
expectedInitialEvents: r'''
46024+
[operation] linkLibraryCycle SDK
46025+
[operation] linkLibraryCycle
46026+
package:test/test.dart
46027+
declaredClasses
46028+
A: #M0
46029+
interface: #M1
46030+
B: #M2
46031+
interface: #M3
46032+
C: #M4
46033+
interface: #M5
46034+
D: #M6
46035+
interface: #M7
46036+
''',
46037+
updatedCode: r'''
46038+
class A<in T> {}
46039+
class B<out T> {}
46040+
class C<out T> {}
46041+
class D<in T> {}
46042+
''',
46043+
expectedUpdatedEvents: r'''
46044+
[operation] linkLibraryCycle
46045+
package:test/test.dart
46046+
declaredClasses
46047+
A: #M0
46048+
interface: #M1
46049+
B: #M2
46050+
interface: #M3
46051+
C: #M8
46052+
interface: #M9
46053+
D: #M10
46054+
interface: #M11
46055+
''',
46056+
);
46057+
}
46058+
46059+
test_manifest_class_typeParameters_variance_legacyCovariant() async {
46060+
await _runLibraryManifestScenario(
46061+
initialCode: r'''
46062+
class A<T> {}
46063+
class B<out T> {}
46064+
''',
46065+
expectedInitialEvents: r'''
46066+
[operation] linkLibraryCycle SDK
46067+
[operation] linkLibraryCycle
46068+
package:test/test.dart
46069+
declaredClasses
46070+
A: #M0
46071+
interface: #M1
46072+
B: #M2
46073+
interface: #M3
46074+
''',
46075+
updatedCode: r'''
46076+
class A<out T> {}
46077+
class B<T> {}
46078+
''',
46079+
expectedUpdatedEvents: r'''
46080+
[operation] linkLibraryCycle
46081+
package:test/test.dart
46082+
declaredClasses
46083+
A: #M0
46084+
interface: #M1
46085+
B: #M2
46086+
interface: #M3
46087+
''',
46088+
);
46089+
}
46090+
4601146091
test_manifest_classTypeAlias_constructors_add() async {
4601246092
await _runLibraryManifestScenario(
4601346093
initialCode: r'''
@@ -53238,14 +53318,16 @@ mixin A<T> {
5323853318
declaredMixins
5323953319
A: #M0
5324053320
typeParameters
53241-
bound: <null>
53321+
#0 covariant
53322+
bound: <null>
5324253323
superclassConstraints
5324353324
Object @ dart:core
5324453325
declaredMethods
5324553326
foo: #M1
5324653327
functionType: FunctionType
5324753328
typeParameters
53248-
bound: <null>
53329+
#0 covariant
53330+
bound: <null>
5324953331
returnType: Map @ dart:core
5325053332
typeParameter#1
5325153333
typeParameter#0
@@ -53265,7 +53347,8 @@ mixin A<T> {
5326553347
declaredMixins
5326653348
A: #M0
5326753349
typeParameters
53268-
bound: <null>
53350+
#0 covariant
53351+
bound: <null>
5326953352
superclassConstraints
5327053353
Object @ dart:core
5327153354
declaredMethods
@@ -53275,7 +53358,8 @@ mixin A<T> {
5327553358
foo: #M1
5327653359
functionType: FunctionType
5327753360
typeParameters
53278-
bound: <null>
53361+
#0 covariant
53362+
bound: <null>
5327953363
returnType: Map @ dart:core
5328053364
typeParameter#1
5328153365
typeParameter#0
@@ -54182,6 +54266,50 @@ mixin A<T, U> {}
5418254266
);
5418354267
}
5418454268

54269+
test_manifest_mixin_typeParameters_variance() async {
54270+
await _runLibraryManifestScenario(
54271+
initialCode: r'''
54272+
mixin A<in T> {}
54273+
mixin B<out T> {}
54274+
mixin C<in T> {}
54275+
mixin D<out T> {}
54276+
''',
54277+
expectedInitialEvents: r'''
54278+
[operation] linkLibraryCycle SDK
54279+
[operation] linkLibraryCycle
54280+
package:test/test.dart
54281+
declaredMixins
54282+
A: #M0
54283+
interface: #M1
54284+
B: #M2
54285+
interface: #M3
54286+
C: #M4
54287+
interface: #M5
54288+
D: #M6
54289+
interface: #M7
54290+
''',
54291+
updatedCode: r'''
54292+
mixin A<in T> {}
54293+
mixin B<out T> {}
54294+
mixin C<out T> {}
54295+
mixin D<in T> {}
54296+
''',
54297+
expectedUpdatedEvents: r'''
54298+
[operation] linkLibraryCycle
54299+
package:test/test.dart
54300+
declaredMixins
54301+
A: #M0
54302+
interface: #M1
54303+
B: #M2
54304+
interface: #M3
54305+
C: #M8
54306+
interface: #M9
54307+
D: #M10
54308+
interface: #M11
54309+
''',
54310+
);
54311+
}
54312+
5418554313
test_manifest_topLevelFunction_add() async {
5418654314
await _runLibraryManifestScenario(
5418754315
initialCode: r'''
@@ -57207,6 +57335,65 @@ typedef B = int;
5720757335
);
5720857336
}
5720957337

57338+
test_manifest_typeAlias_typeParameters_bound() async {
57339+
configuration.withElementManifests = true;
57340+
await _runLibraryManifestScenario(
57341+
initialCode: r'''
57342+
typedef F<X extends num> = List<X>;
57343+
''',
57344+
expectedInitialEvents: r'''
57345+
[operation] linkLibraryCycle SDK
57346+
[operation] linkLibraryCycle
57347+
package:test/test.dart
57348+
declaredTypeAliases
57349+
F: #M0
57350+
typeParameters
57351+
#0 covariant
57352+
bound: num @ dart:core
57353+
aliasedType: List @ dart:core
57354+
typeParameter#0
57355+
''',
57356+
updatedCode: r'''
57357+
typedef F<X extends int> = List<X>;
57358+
''',
57359+
expectedUpdatedEvents: r'''
57360+
[operation] linkLibraryCycle
57361+
package:test/test.dart
57362+
declaredTypeAliases
57363+
F: #M1
57364+
typeParameters
57365+
#0 covariant
57366+
bound: int @ dart:core
57367+
aliasedType: List @ dart:core
57368+
typeParameter#0
57369+
''',
57370+
);
57371+
}
57372+
57373+
test_manifest_typeAlias_typeParameters_name() async {
57374+
await _runLibraryManifestScenario(
57375+
initialCode: r'''
57376+
typedef F<X> = List<X>;
57377+
''',
57378+
expectedInitialEvents: r'''
57379+
[operation] linkLibraryCycle SDK
57380+
[operation] linkLibraryCycle
57381+
package:test/test.dart
57382+
declaredTypeAliases
57383+
F: #M0
57384+
''',
57385+
updatedCode: r'''
57386+
typedef F<Y> = List<Y>;
57387+
''',
57388+
expectedUpdatedEvents: r'''
57389+
[operation] linkLibraryCycle
57390+
package:test/test.dart
57391+
declaredTypeAliases
57392+
F: #M0
57393+
''',
57394+
);
57395+
}
57396+
5721057397
test_operation_addFile_changeImported_affected() async {
5721157398
configuration.withCheckLibraryDiagnosticsRequirements = true;
5721257399

pkg/analyzer/test/src/dart/analysis/result_printer.dart

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,8 +1567,16 @@ class LibraryManifestPrinter {
15671567
}
15681568

15691569
void _writeTypeParameters(List<ManifestTypeParameter> typeParameters) {
1570-
sink.writeElements('typeParameters', typeParameters, (typeParameter) {
1571-
_writeNamedType('bound', typeParameter.bound);
1570+
var indexed = typeParameters.indexed.toList();
1571+
sink.writeElements('typeParameters', indexed, (pair) {
1572+
var typeParameter = pair.$2;
1573+
sink.writeIndentedLine(() {
1574+
sink.write('#${pair.$1} ');
1575+
sink.write(typeParameter.variance.name);
1576+
});
1577+
sink.withIndent(() {
1578+
_writeNamedType('bound', typeParameter.bound);
1579+
});
15721580
});
15731581
}
15741582
}

0 commit comments

Comments
 (0)