Skip to content

Commit

Permalink
[cfe] Check overrides on operators
Browse files Browse the repository at this point in the history
Change-Id: Ib250117553b2e22c6c71b78bf354be1162bc9981
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151235
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
  • Loading branch information
johnniwinther authored and commit-bot@chromium.org committed Jul 8, 2020
1 parent fefcac4 commit 45af991
Show file tree
Hide file tree
Showing 32 changed files with 965 additions and 37 deletions.
83 changes: 58 additions & 25 deletions pkg/front_end/lib/src/fasta/builder/class_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ import '../loader.dart';

import '../modifier.dart';

import '../names.dart' show noSuchMethodName;
import '../names.dart' show equalsName, noSuchMethodName;

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

Expand Down Expand Up @@ -791,27 +791,30 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
"Constructor in override check.", declaredMember.fileOffset, fileUri);
}
if (declaredMember is Procedure && interfaceMember is Procedure) {
if (declaredMember.kind == ProcedureKind.Method &&
interfaceMember.kind == ProcedureKind.Method) {
bool seenCovariant = checkMethodOverride(
types, declaredMember, interfaceMember, isInterfaceCheck);
if (seenCovariant) {
handleSeenCovariant(
types, declaredMember, interfaceMember, isSetter, callback);
}
}
if (declaredMember.kind == ProcedureKind.Getter &&
interfaceMember.kind == ProcedureKind.Getter) {
checkGetterOverride(
types, declaredMember, interfaceMember, isInterfaceCheck);
}
if (declaredMember.kind == ProcedureKind.Setter &&
interfaceMember.kind == ProcedureKind.Setter) {
bool seenCovariant = checkSetterOverride(
types, declaredMember, interfaceMember, isInterfaceCheck);
if (seenCovariant) {
handleSeenCovariant(
types, declaredMember, interfaceMember, isSetter, callback);
if (declaredMember.kind == interfaceMember.kind) {
if (declaredMember.kind == ProcedureKind.Method ||
declaredMember.kind == ProcedureKind.Operator) {
bool seenCovariant = checkMethodOverride(
types, declaredMember, interfaceMember, isInterfaceCheck);
if (seenCovariant) {
handleSeenCovariant(
types, declaredMember, interfaceMember, isSetter, callback);
}
} else if (declaredMember.kind == ProcedureKind.Getter) {
checkGetterOverride(
types, declaredMember, interfaceMember, isInterfaceCheck);
} else if (declaredMember.kind == ProcedureKind.Setter) {
bool seenCovariant = checkSetterOverride(
types, declaredMember, interfaceMember, isInterfaceCheck);
if (seenCovariant) {
handleSeenCovariant(
types, declaredMember, interfaceMember, isSetter, callback);
}
} else {
assert(
false,
"Unexpected procedure kind in override check: "
"${declaredMember.kind}");
}
}
} else {
Expand Down Expand Up @@ -1093,8 +1096,9 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
@override
bool checkMethodOverride(Types types, Procedure declaredMember,
Procedure interfaceMember, bool isInterfaceCheck) {
assert(declaredMember.kind == ProcedureKind.Method);
assert(interfaceMember.kind == ProcedureKind.Method);
assert(declaredMember.kind == interfaceMember.kind);
assert(declaredMember.kind == ProcedureKind.Method ||
declaredMember.kind == ProcedureKind.Operator);
bool seenCovariant = false;
FunctionNode declaredFunction = declaredMember.function;
FunctionNode interfaceFunction = interfaceMember.function;
Expand Down Expand Up @@ -1167,14 +1171,25 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
declaredFunction.positionalParameters[i];
VariableDeclaration interfaceParameter =
interfaceFunction.positionalParameters[i];
if (i == 0 &&
declaredMember.name == equalsName &&
declaredParameter.type ==
types.hierarchy.coreTypes.objectNonNullableRawType &&
interfaceParameter.type is DynamicType) {
// TODO(johnniwinther): Add check for opt-in overrides of operator ==.
// `operator ==` methods in opt-out classes have type
// `bool Function(dynamic)`.
continue;
}

_checkTypes(
types,
interfaceSubstitution,
declaredSubstitution,
declaredMember,
interfaceMember,
declaredParameter.type,
interfaceFunction.positionalParameters[i].type,
interfaceParameter.type,
declaredParameter.isCovariant || interfaceParameter.isCovariant,
declaredParameter,
isInterfaceCheck);
Expand Down Expand Up @@ -1340,6 +1355,10 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
void reportInvalidOverride(bool isInterfaceCheck, Member declaredMember,
Message message, int fileOffset, int length,
{List<LocatedMessage> context}) {
if (shouldOverrideProblemBeOverlooked(this)) {
return;
}

if (declaredMember.enclosingClass == cls) {
// Ordinary override
library.addProblem(message, fileOffset, length, declaredMember.fileUri,
Expand Down Expand Up @@ -1769,3 +1788,17 @@ class ConstructorRedirection {

ConstructorRedirection(this.target) : cycleReported = false;
}

/// Returns `true` if override problems should be overlooked.
///
/// This is needed for the current encoding of some JavaScript implementation
/// classes that are not valid Dart. For instance `JSInt` in
/// 'dart:_interceptors' that implements both `int` and `double`, and `JsArray`
/// in `dart:js` that implement both `ListMixin` and `JsObject`.
bool shouldOverrideProblemBeOverlooked(ClassBuilder classBuilder) {
String uri = '${classBuilder.library.importUri}';
return uri == 'dart:js' &&
classBuilder.fileUri.pathSegments.last == 'js.dart' ||
uri == 'dart:_interceptors' &&
classBuilder.fileUri.pathSegments.last == 'js_number.dart';
}
Original file line number Diff line number Diff line change
Expand Up @@ -3038,14 +3038,7 @@ class InterfaceConflict extends DelayedMember {
debug?.log("Combined Member Signature: "
"${bestSoFar.fullName} !<: ${candidate.fullName}");

String uri = '${classBuilder.library.importUri}';
if (uri == 'dart:js' &&
classBuilder.fileUri.pathSegments.last == 'js.dart' ||
uri == 'dart:_interceptors' &&
classBuilder.fileUri.pathSegments.last == 'js_number.dart') {
// TODO(johnniwinther): Fix the dart2js libraries and remove the
// above URIs.
} else {
if (!shouldOverrideProblemBeOverlooked(classBuilder)) {
bestSoFar = null;
bestTypeSoFar = null;
mutualSubtypes = null;
Expand Down
1 change: 1 addition & 0 deletions pkg/front_end/test/spell_checking_list_code.txt
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,7 @@ os
outputting
overlap
overloader
overlooked
overshadowed
overwrite
overwriting
Expand Down
24 changes: 24 additions & 0 deletions pkg/front_end/testcases/general/abstract_operator_override.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) 2020, 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.

class A {
A operator +(B b) => new A();
A operator -() => new A();
A operator [](B b) => new A();
void operator []=(B b1, B b2) {}
}

class B extends A {
A operator +(A a);
B operator -();
A operator [](A a);
void operator []=(A a, B b);
}

class C extends A {
B operator [](B b);
void operator []=(B b, A a);
}

main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
library;
//
// Problems in library:
//
// 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.
// class B extends A {
// ^
// 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.+'.
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// Change to a supertype of 'A', or, for a covariant parameter, a subtype.
// A operator +(B b) => new A();
// ^
// pkg/front_end/testcases/general/abstract_operator_override.dart:13:14: Context: This is the overridden method ('+').
// A operator +(A a);
// ^
//
// 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.
// class B extends A {
// ^
// 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-'.
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// Change to a subtype of 'B'.
// A operator -() => new A();
// ^
// pkg/front_end/testcases/general/abstract_operator_override.dart:14:14: Context: This is the overridden method ('unary-').
// B operator -();
// ^
//
// 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.
// class B extends A {
// ^
// 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.[]'.
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// Change to a supertype of 'A', or, for a covariant parameter, a subtype.
// A operator [](B b) => new A();
// ^
// pkg/front_end/testcases/general/abstract_operator_override.dart:15:14: Context: This is the overridden method ('[]').
// A operator [](A a);
// ^
//
// 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.
// class B extends A {
// ^
// 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.[]='.
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// Change to a supertype of 'A', or, for a covariant parameter, a subtype.
// void operator []=(B b1, B b2) {}
// ^
// pkg/front_end/testcases/general/abstract_operator_override.dart:16:17: Context: This is the overridden method ('[]=').
// void operator []=(A a, B b);
// ^
//
// 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.
// class C extends A {
// ^
// 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.[]'.
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// Change to a subtype of 'B'.
// A operator [](B b) => new A();
// ^
// pkg/front_end/testcases/general/abstract_operator_override.dart:20:14: Context: This is the overridden method ('[]').
// B operator [](B b);
// ^
//
// 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.
// class C extends A {
// ^
// 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.[]='.
// - 'B' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// - 'A' is from 'pkg/front_end/testcases/general/abstract_operator_override.dart'.
// Change to a supertype of 'A', or, for a covariant parameter, a subtype.
// void operator []=(B b1, B b2) {}
// ^
// pkg/front_end/testcases/general/abstract_operator_override.dart:21:17: Context: This is the overridden method ('[]=').
// void operator []=(B b, A a);
// ^
//
import self as self;
import "dart:core" as core;

class A extends core::Object {
synthetic constructor •() → self::A*
;
operator +(self::B* b) → self::A*
;
operator unary-() → self::A*
;
operator [](self::B* b) → self::A*
;
operator []=(self::B* b1, self::B* b2) → void
;
abstract member-signature get _identityHashCode() → core::int*;
abstract member-signature method _instanceOf(dynamic instantiatorTypeArguments, dynamic functionTypeArguments, dynamic type) → core::bool*;
abstract member-signature method _simpleInstanceOf(dynamic type) → core::bool*;
abstract member-signature method _simpleInstanceOfTrue(dynamic type) → core::bool*;
abstract member-signature method _simpleInstanceOfFalse(dynamic type) → core::bool*;
abstract member-signature operator ==(dynamic other) → core::bool*;
abstract member-signature get hashCode() → core::int*;
abstract member-signature method toString() → core::String*;
abstract member-signature method noSuchMethod(core::Invocation* invocation) → dynamic;
abstract member-signature get runtimeType() → core::Type*;
}
class B extends self::A {
synthetic constructor •() → self::B*
;
abstract operator +(self::A* a) → self::A*;
abstract operator unary-() → self::B*;
abstract operator [](self::A* a) → self::A*;
abstract operator []=(self::A* a, self::B* b) → void;
}
class C extends self::A {
synthetic constructor •() → self::C*
;
abstract operator [](self::B* b) → self::B*;
abstract operator []=(self::B* b, self::A* a) → void;
}
static method main() → dynamic
;
Loading

0 comments on commit 45af991

Please sign in to comment.