Skip to content

Commit 45af991

Browse files
johnniwinthercommit-bot@chromium.org
johnniwinther
authored and
[email protected]
committed
[cfe] Check overrides on operators
Change-Id: Ib250117553b2e22c6c71b78bf354be1162bc9981 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151235 Reviewed-by: Dmitry Stefantsov <[email protected]> Commit-Queue: Johnni Winther <[email protected]>
1 parent fefcac4 commit 45af991

32 files changed

+965
-37
lines changed

‎pkg/front_end/lib/src/fasta/builder/class_builder.dart

+58-25
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ import '../loader.dart';
6262

6363
import '../modifier.dart';
6464

65-
import '../names.dart' show noSuchMethodName;
65+
import '../names.dart' show equalsName, noSuchMethodName;
6666

6767
import '../problems.dart' show internalProblem, unhandled, unimplemented;
6868

@@ -791,27 +791,30 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
791791
"Constructor in override check.", declaredMember.fileOffset, fileUri);
792792
}
793793
if (declaredMember is Procedure && interfaceMember is Procedure) {
794-
if (declaredMember.kind == ProcedureKind.Method &&
795-
interfaceMember.kind == ProcedureKind.Method) {
796-
bool seenCovariant = checkMethodOverride(
797-
types, declaredMember, interfaceMember, isInterfaceCheck);
798-
if (seenCovariant) {
799-
handleSeenCovariant(
800-
types, declaredMember, interfaceMember, isSetter, callback);
801-
}
802-
}
803-
if (declaredMember.kind == ProcedureKind.Getter &&
804-
interfaceMember.kind == ProcedureKind.Getter) {
805-
checkGetterOverride(
806-
types, declaredMember, interfaceMember, isInterfaceCheck);
807-
}
808-
if (declaredMember.kind == ProcedureKind.Setter &&
809-
interfaceMember.kind == ProcedureKind.Setter) {
810-
bool seenCovariant = checkSetterOverride(
811-
types, declaredMember, interfaceMember, isInterfaceCheck);
812-
if (seenCovariant) {
813-
handleSeenCovariant(
814-
types, declaredMember, interfaceMember, isSetter, callback);
794+
if (declaredMember.kind == interfaceMember.kind) {
795+
if (declaredMember.kind == ProcedureKind.Method ||
796+
declaredMember.kind == ProcedureKind.Operator) {
797+
bool seenCovariant = checkMethodOverride(
798+
types, declaredMember, interfaceMember, isInterfaceCheck);
799+
if (seenCovariant) {
800+
handleSeenCovariant(
801+
types, declaredMember, interfaceMember, isSetter, callback);
802+
}
803+
} else if (declaredMember.kind == ProcedureKind.Getter) {
804+
checkGetterOverride(
805+
types, declaredMember, interfaceMember, isInterfaceCheck);
806+
} else if (declaredMember.kind == ProcedureKind.Setter) {
807+
bool seenCovariant = checkSetterOverride(
808+
types, declaredMember, interfaceMember, isInterfaceCheck);
809+
if (seenCovariant) {
810+
handleSeenCovariant(
811+
types, declaredMember, interfaceMember, isSetter, callback);
812+
}
813+
} else {
814+
assert(
815+
false,
816+
"Unexpected procedure kind in override check: "
817+
"${declaredMember.kind}");
815818
}
816819
}
817820
} else {
@@ -1093,8 +1096,9 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
10931096
@override
10941097
bool checkMethodOverride(Types types, Procedure declaredMember,
10951098
Procedure interfaceMember, bool isInterfaceCheck) {
1096-
assert(declaredMember.kind == ProcedureKind.Method);
1097-
assert(interfaceMember.kind == ProcedureKind.Method);
1099+
assert(declaredMember.kind == interfaceMember.kind);
1100+
assert(declaredMember.kind == ProcedureKind.Method ||
1101+
declaredMember.kind == ProcedureKind.Operator);
10981102
bool seenCovariant = false;
10991103
FunctionNode declaredFunction = declaredMember.function;
11001104
FunctionNode interfaceFunction = interfaceMember.function;
@@ -1167,14 +1171,25 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
11671171
declaredFunction.positionalParameters[i];
11681172
VariableDeclaration interfaceParameter =
11691173
interfaceFunction.positionalParameters[i];
1174+
if (i == 0 &&
1175+
declaredMember.name == equalsName &&
1176+
declaredParameter.type ==
1177+
types.hierarchy.coreTypes.objectNonNullableRawType &&
1178+
interfaceParameter.type is DynamicType) {
1179+
// TODO(johnniwinther): Add check for opt-in overrides of operator ==.
1180+
// `operator ==` methods in opt-out classes have type
1181+
// `bool Function(dynamic)`.
1182+
continue;
1183+
}
1184+
11701185
_checkTypes(
11711186
types,
11721187
interfaceSubstitution,
11731188
declaredSubstitution,
11741189
declaredMember,
11751190
interfaceMember,
11761191
declaredParameter.type,
1177-
interfaceFunction.positionalParameters[i].type,
1192+
interfaceParameter.type,
11781193
declaredParameter.isCovariant || interfaceParameter.isCovariant,
11791194
declaredParameter,
11801195
isInterfaceCheck);
@@ -1340,6 +1355,10 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
13401355
void reportInvalidOverride(bool isInterfaceCheck, Member declaredMember,
13411356
Message message, int fileOffset, int length,
13421357
{List<LocatedMessage> context}) {
1358+
if (shouldOverrideProblemBeOverlooked(this)) {
1359+
return;
1360+
}
1361+
13431362
if (declaredMember.enclosingClass == cls) {
13441363
// Ordinary override
13451364
library.addProblem(message, fileOffset, length, declaredMember.fileUri,
@@ -1769,3 +1788,17 @@ class ConstructorRedirection {
17691788

17701789
ConstructorRedirection(this.target) : cycleReported = false;
17711790
}
1791+
1792+
/// Returns `true` if override problems should be overlooked.
1793+
///
1794+
/// This is needed for the current encoding of some JavaScript implementation
1795+
/// classes that are not valid Dart. For instance `JSInt` in
1796+
/// 'dart:_interceptors' that implements both `int` and `double`, and `JsArray`
1797+
/// in `dart:js` that implement both `ListMixin` and `JsObject`.
1798+
bool shouldOverrideProblemBeOverlooked(ClassBuilder classBuilder) {
1799+
String uri = '${classBuilder.library.importUri}';
1800+
return uri == 'dart:js' &&
1801+
classBuilder.fileUri.pathSegments.last == 'js.dart' ||
1802+
uri == 'dart:_interceptors' &&
1803+
classBuilder.fileUri.pathSegments.last == 'js_number.dart';
1804+
}

‎pkg/front_end/lib/src/fasta/kernel/class_hierarchy_builder.dart

+1-8
Original file line numberDiff line numberDiff line change
@@ -3038,14 +3038,7 @@ class InterfaceConflict extends DelayedMember {
30383038
debug?.log("Combined Member Signature: "
30393039
"${bestSoFar.fullName} !<: ${candidate.fullName}");
30403040

3041-
String uri = '${classBuilder.library.importUri}';
3042-
if (uri == 'dart:js' &&
3043-
classBuilder.fileUri.pathSegments.last == 'js.dart' ||
3044-
uri == 'dart:_interceptors' &&
3045-
classBuilder.fileUri.pathSegments.last == 'js_number.dart') {
3046-
// TODO(johnniwinther): Fix the dart2js libraries and remove the
3047-
// above URIs.
3048-
} else {
3041+
if (!shouldOverrideProblemBeOverlooked(classBuilder)) {
30493042
bestSoFar = null;
30503043
bestTypeSoFar = null;
30513044
mutualSubtypes = null;

‎pkg/front_end/test/spell_checking_list_code.txt

+1
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,7 @@ os
751751
outputting
752752
overlap
753753
overloader
754+
overlooked
754755
overshadowed
755756
overwrite
756757
overwriting

‎pkg/front_end/testcases/general/abstract_operator_override.dart

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
class A {
6+
A operator +(B b) => new A();
7+
A operator -() => new A();
8+
A operator [](B b) => new A();
9+
void operator []=(B b1, B b2) {}
10+
}
11+
12+
class B extends A {
13+
A operator +(A a);
14+
B operator -();
15+
A operator [](A a);
16+
void operator []=(A a, B b);
17+
}
18+
19+
class C extends A {
20+
B operator [](B b);
21+
void operator []=(B b, A a);
22+
}
23+
24+
main() {}

‎pkg/front_end/testcases/general/abstract_operator_override.dart.outline.expect

+123
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
library;
2+
//
3+
// Problems in library:
4+
//
5+
// pkg/front_end/testcases/general/abstract_operator_override.dart:12:7: Error: The implementation of '+' in the non-abstract class 'B' does not conform to its interface.
6+
// class B extends A {
7+
// ^
8+
// pkg/front_end/testcases/general/abstract_operator_override.dart:6:18: Context: The parameter 'b' of the method 'A.+' has type 'B', which does not match the corresponding type, 'A', in the overridden method, 'B.+'.
9+
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
10+
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
11+
// Change to a supertype of 'A', or, for a covariant parameter, a subtype.
12+
// A operator +(B b) => new A();
13+
// ^
14+
// pkg/front_end/testcases/general/abstract_operator_override.dart:13:14: Context: This is the overridden method ('+').
15+
// A operator +(A a);
16+
// ^
17+
//
18+
// pkg/front_end/testcases/general/abstract_operator_override.dart:12:7: Error: The implementation of 'unary-' in the non-abstract class 'B' does not conform to its interface.
19+
// class B extends A {
20+
// ^
21+
// pkg/front_end/testcases/general/abstract_operator_override.dart:7:14: Context: The return type of the method 'A.unary-' is 'A', which does not match the return type, 'B', of the overridden method, 'B.unary-'.
22+
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
23+
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
24+
// Change to a subtype of 'B'.
25+
// A operator -() => new A();
26+
// ^
27+
// pkg/front_end/testcases/general/abstract_operator_override.dart:14:14: Context: This is the overridden method ('unary-').
28+
// B operator -();
29+
// ^
30+
//
31+
// pkg/front_end/testcases/general/abstract_operator_override.dart:12:7: Error: The implementation of '[]' in the non-abstract class 'B' does not conform to its interface.
32+
// class B extends A {
33+
// ^
34+
// pkg/front_end/testcases/general/abstract_operator_override.dart:8:19: Context: The parameter 'b' of the method 'A.[]' has type 'B', which does not match the corresponding type, 'A', in the overridden method, 'B.[]'.
35+
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
36+
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
37+
// Change to a supertype of 'A', or, for a covariant parameter, a subtype.
38+
// A operator [](B b) => new A();
39+
// ^
40+
// pkg/front_end/testcases/general/abstract_operator_override.dart:15:14: Context: This is the overridden method ('[]').
41+
// A operator [](A a);
42+
// ^
43+
//
44+
// pkg/front_end/testcases/general/abstract_operator_override.dart:12:7: Error: The implementation of '[]=' in the non-abstract class 'B' does not conform to its interface.
45+
// class B extends A {
46+
// ^
47+
// pkg/front_end/testcases/general/abstract_operator_override.dart:9:23: Context: The parameter 'b1' of the method 'A.[]=' has type 'B', which does not match the corresponding type, 'A', in the overridden method, 'B.[]='.
48+
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
49+
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
50+
// Change to a supertype of 'A', or, for a covariant parameter, a subtype.
51+
// void operator []=(B b1, B b2) {}
52+
// ^
53+
// pkg/front_end/testcases/general/abstract_operator_override.dart:16:17: Context: This is the overridden method ('[]=').
54+
// void operator []=(A a, B b);
55+
// ^
56+
//
57+
// pkg/front_end/testcases/general/abstract_operator_override.dart:19:7: Error: The implementation of '[]' in the non-abstract class 'C' does not conform to its interface.
58+
// class C extends A {
59+
// ^
60+
// pkg/front_end/testcases/general/abstract_operator_override.dart:8:14: Context: The return type of the method 'A.[]' is 'A', which does not match the return type, 'B', of the overridden method, 'C.[]'.
61+
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
62+
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
63+
// Change to a subtype of 'B'.
64+
// A operator [](B b) => new A();
65+
// ^
66+
// pkg/front_end/testcases/general/abstract_operator_override.dart:20:14: Context: This is the overridden method ('[]').
67+
// B operator [](B b);
68+
// ^
69+
//
70+
// pkg/front_end/testcases/general/abstract_operator_override.dart:19:7: Error: The implementation of '[]=' in the non-abstract class 'C' does not conform to its interface.
71+
// class C extends A {
72+
// ^
73+
// pkg/front_end/testcases/general/abstract_operator_override.dart:9:29: Context: The parameter 'b2' of the method 'A.[]=' has type 'B', which does not match the corresponding type, 'A', in the overridden method, 'C.[]='.
74+
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
75+
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
76+
// Change to a supertype of 'A', or, for a covariant parameter, a subtype.
77+
// void operator []=(B b1, B b2) {}
78+
// ^
79+
// pkg/front_end/testcases/general/abstract_operator_override.dart:21:17: Context: This is the overridden method ('[]=').
80+
// void operator []=(B b, A a);
81+
// ^
82+
//
83+
import self as self;
84+
import "dart:core" as core;
85+
86+
class A extends core::Object {
87+
synthetic constructor •() → self::A*
88+
;
89+
operator +(self::B* b) → self::A*
90+
;
91+
operator unary-() → self::A*
92+
;
93+
operator [](self::B* b) → self::A*
94+
;
95+
operator []=(self::B* b1, self::B* b2) → void
96+
;
97+
abstract member-signature get _identityHashCode() → core::int*;
98+
abstract member-signature method _instanceOf(dynamic instantiatorTypeArguments, dynamic functionTypeArguments, dynamic type) → core::bool*;
99+
abstract member-signature method _simpleInstanceOf(dynamic type) → core::bool*;
100+
abstract member-signature method _simpleInstanceOfTrue(dynamic type) → core::bool*;
101+
abstract member-signature method _simpleInstanceOfFalse(dynamic type) → core::bool*;
102+
abstract member-signature operator ==(dynamic other) → core::bool*;
103+
abstract member-signature get hashCode() → core::int*;
104+
abstract member-signature method toString() → core::String*;
105+
abstract member-signature method noSuchMethod(core::Invocation* invocation) → dynamic;
106+
abstract member-signature get runtimeType() → core::Type*;
107+
}
108+
class B extends self::A {
109+
synthetic constructor •() → self::B*
110+
;
111+
abstract operator +(self::A* a) → self::A*;
112+
abstract operator unary-() → self::B*;
113+
abstract operator [](self::A* a) → self::A*;
114+
abstract operator []=(self::A* a, self::B* b) → void;
115+
}
116+
class C extends self::A {
117+
synthetic constructor •() → self::C*
118+
;
119+
abstract operator [](self::B* b) → self::B*;
120+
abstract operator []=(self::B* b, self::A* a) → void;
121+
}
122+
static method main() → dynamic
123+
;

0 commit comments

Comments
 (0)