Skip to content

Commit

Permalink
Fix #28120, strong mode allows field overrides
Browse files Browse the repository at this point in the history
Fix #28119, DDC supports field overrides without @virtual
Fix #28801, devirtualize private fields in DDC where possible
Fix #28589, stop supporting @virtual in strong mode

R=leafp@google.com, vsm@google.com

Review-Url: https://codereview.chromium.org/2781443003 .
  • Loading branch information
Jennifer Messerly committed Mar 28, 2017
1 parent 9dd0668 commit 1c504f8
Show file tree
Hide file tree
Showing 24 changed files with 19,009 additions and 5,949 deletions.
27 changes: 1 addition & 26 deletions pkg/analyzer/lib/src/dart/element/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2520,12 +2520,6 @@ class ElementAnnotationImpl implements ElementAnnotation {
*/
static String _REQUIRED_VARIABLE_NAME = "required";

/**
* The name of the top-level variable used to mark a member as intended to be
* overridden.
*/
static String _VIRTUAL_VARIABLE_NAME = "virtual";

/**
* The element representing the field, variable, or constructor being used as
* an annotation.
Expand Down Expand Up @@ -2634,18 +2628,6 @@ class ElementAnnotationImpl implements ElementAnnotation {
element.name == _REQUIRED_VARIABLE_NAME &&
element.library?.name == _META_LIB_NAME;

/**
* Return `true` if this annotation marks the associated member as supporting
* overrides.
*
* This is currently used by fields in Strong Mode, as other members are
* already virtual-by-default.
*/
bool get isVirtual =>
element is PropertyAccessorElement &&
element.name == _VIRTUAL_VARIABLE_NAME &&
element.library?.name == _META_LIB_NAME;

/**
* Get the library containing this annotation.
*/
Expand Down Expand Up @@ -4273,14 +4255,7 @@ class FieldElementImpl extends PropertyInducingElementImpl
}

@override
bool get isVirtual {
for (ElementAnnotationImpl annotation in metadata) {
if (annotation.isVirtual) {
return true;
}
}
return false;
}
bool get isVirtual => true;

@override
ElementKind get kind => ElementKind.FIELD;
Expand Down
18 changes: 6 additions & 12 deletions pkg/analyzer/test/generated/compile_time_error_code_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5291,10 +5291,8 @@ import 'lib1.dart';
class C extends Object with A, B {}
''');
await computeAnalysisResult(source);
assertErrors(source, [
CompileTimeErrorCode.PRIVATE_COLLISION_IN_MIXIN_APPLICATION,
StrongModeCode.INVALID_FIELD_OVERRIDE
]);
assertErrors(
source, [CompileTimeErrorCode.PRIVATE_COLLISION_IN_MIXIN_APPLICATION]);
verify([source]);
}

Expand Down Expand Up @@ -5340,10 +5338,8 @@ import 'lib1.dart';
class C extends A with B {}
''');
await computeAnalysisResult(source);
assertErrors(source, [
CompileTimeErrorCode.PRIVATE_COLLISION_IN_MIXIN_APPLICATION,
StrongModeCode.INVALID_FIELD_OVERRIDE
]);
assertErrors(
source, [CompileTimeErrorCode.PRIVATE_COLLISION_IN_MIXIN_APPLICATION]);
verify([source]);
}

Expand All @@ -5365,10 +5361,8 @@ import 'lib1.dart';
class C extends A with A {}
''');
await computeAnalysisResult(source);
assertErrors(source, [
CompileTimeErrorCode.PRIVATE_COLLISION_IN_MIXIN_APPLICATION,
StrongModeCode.INVALID_FIELD_OVERRIDE
]);
assertErrors(
source, [CompileTimeErrorCode.PRIVATE_COLLISION_IN_MIXIN_APPLICATION]);
verify([source]);
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/analyzer/test/src/context/context_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4326,9 +4326,8 @@ class B extends A {
}
''');
_performPendingAnalysisTasks();
expect(context.getErrors(a).errors, hasLength(2));
expect(context.getErrors(a).errors, hasLength(1));
// Update a.dart: rename "int foo" to "int bar".
// The strong mode "getter cannot override field" error is gone.
context.setContents(
a,
r'''
Expand Down
64 changes: 32 additions & 32 deletions pkg/analyzer/test/src/task/strong/checker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -726,10 +726,10 @@ class Base {
}
class Child extends Base {
/*error:INVALID_FIELD_OVERRIDE,error:INVALID_METHOD_OVERRIDE*/A f1; // invalid for getter
/*error:INVALID_FIELD_OVERRIDE,error:INVALID_METHOD_OVERRIDE*/C f2; // invalid for setter
/*error:INVALID_FIELD_OVERRIDE*/var f3;
/*error:INVALID_FIELD_OVERRIDE,error:INVALID_METHOD_OVERRIDE*/dynamic f4;
/*error:INVALID_METHOD_OVERRIDE*/A f1; // invalid for getter
/*error:INVALID_METHOD_OVERRIDE*/C f2; // invalid for setter
var f3;
/*error:INVALID_METHOD_OVERRIDE*/dynamic f4;
}
class Child2 implements Base {
Expand All @@ -755,10 +755,10 @@ abstract class Base {
}
class Child extends Base {
/*error:INVALID_FIELD_OVERRIDE,error:INVALID_METHOD_OVERRIDE*/A get f1 => null;
/*error:INVALID_FIELD_OVERRIDE*/C get f2 => null;
/*error:INVALID_FIELD_OVERRIDE*/get f3 => null;
/*error:INVALID_FIELD_OVERRIDE,error:INVALID_METHOD_OVERRIDE*/dynamic get f4 => null;
/*error:INVALID_METHOD_OVERRIDE*/A get f1 => null;
C get f2 => null;
get f3 => null;
/*error:INVALID_METHOD_OVERRIDE*/dynamic get f4 => null;
}
class /*error:NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_FOUR*/Child2 implements Base {
Expand All @@ -779,8 +779,8 @@ class F {
}
class G extends F {
/*error:INVALID_FIELD_OVERRIDE*/final ToVoid<int> f = null;
/*error:INVALID_FIELD_OVERRIDE, error:INVALID_METHOD_OVERRIDE*/final ToVoid<dynamic> g = null;
final ToVoid<int> f = null;
/*error:INVALID_METHOD_OVERRIDE*/final ToVoid<dynamic> g = null;
}
class H implements F {
Expand Down Expand Up @@ -815,7 +815,7 @@ class OverrideWithField extends C {
set superX(int v) { super.x = v; }
}
class VirtualNotInherited extends OverrideWithField {
/*error:INVALID_FIELD_OVERRIDE*/int x;
int x;
}
''');
}
Expand All @@ -835,17 +835,17 @@ class Base {
}
class Child extends Base {
/*error:INVALID_FIELD_OVERRIDE*/B get f1 => null;
/*error:INVALID_FIELD_OVERRIDE*/B get f2 => null;
/*error:INVALID_FIELD_OVERRIDE*/B get f3 => null;
/*error:INVALID_FIELD_OVERRIDE*/B get f4 => null;
/*error:INVALID_FIELD_OVERRIDE*/B get f5 => null;
B get f1 => null;
B get f2 => null;
B get f3 => null;
B get f4 => null;
B get f5 => null;
/*error:INVALID_FIELD_OVERRIDE*/void set f1(A value) {}
/*error:INVALID_FIELD_OVERRIDE,error:INVALID_METHOD_OVERRIDE*/void set f2(C value) {}
/*error:INVALID_FIELD_OVERRIDE*/void set f3(value) {}
/*error:INVALID_FIELD_OVERRIDE*/void set f4(dynamic value) {}
/*error:INVALID_FIELD_OVERRIDE*/set f5(B value) {}
void set f1(A value) {}
/*error:INVALID_METHOD_OVERRIDE*/void set f2(C value) {}
void set f3(value) {}
void set f4(dynamic value) {}
set f5(B value) {}
}
class Child2 implements Base {
Expand Down Expand Up @@ -2494,21 +2494,21 @@ class Base {
}
class T1 extends Base {
/*warning:MISMATCHED_GETTER_AND_SETTER_TYPES_FROM_SUPERTYPE, error:INVALID_FIELD_OVERRIDE, error:INVALID_METHOD_OVERRIDE*/B get f => null;
/*warning:MISMATCHED_GETTER_AND_SETTER_TYPES_FROM_SUPERTYPE,error:INVALID_METHOD_OVERRIDE*/B get f => null;
}
class T2 extends Base {
/*warning:MISMATCHED_GETTER_AND_SETTER_TYPES_FROM_SUPERTYPE, error:INVALID_FIELD_OVERRIDE, error:INVALID_METHOD_OVERRIDE*/set f(
/*warning:MISMATCHED_GETTER_AND_SETTER_TYPES_FROM_SUPERTYPE,error:INVALID_METHOD_OVERRIDE*/set f(
B b) => null;
}
class T3 extends Base {
/*error:INVALID_FIELD_OVERRIDE, error:INVALID_METHOD_OVERRIDE*/final B
/*error:INVALID_METHOD_OVERRIDE*/final B
/*warning:FINAL_NOT_INITIALIZED*/f;
}
class T4 extends Base {
// two: one for the getter one for the setter.
/*error:INVALID_FIELD_OVERRIDE, error:INVALID_METHOD_OVERRIDE, error:INVALID_METHOD_OVERRIDE*/B f;
/*error:INVALID_METHOD_OVERRIDE, error:INVALID_METHOD_OVERRIDE*/B f;
}
class /*error:NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_ONE*/T5 implements Base {
Expand Down Expand Up @@ -2610,7 +2610,7 @@ class Parent extends Grandparent {
class Test extends Parent {
/*error:INVALID_METHOD_OVERRIDE*/m(B a) {}
/*error:INVALID_FIELD_OVERRIDE*/int x;
int x;
}
''');
}
Expand Down Expand Up @@ -2655,9 +2655,9 @@ class M2 {
class /*error:INCONSISTENT_METHOD_INHERITANCE*/T1 extends Base
with /*error:INVALID_METHOD_OVERRIDE_FROM_MIXIN*/M1 {}
class /*error:INCONSISTENT_METHOD_INHERITANCE*/T2 extends Base
with /*error:INVALID_METHOD_OVERRIDE_FROM_MIXIN*/M1, /*error:INVALID_FIELD_OVERRIDE*/M2 {}
with /*error:INVALID_METHOD_OVERRIDE_FROM_MIXIN*/M1, M2 {}
class /*error:INCONSISTENT_METHOD_INHERITANCE*/T3 extends Base
with /*error:INVALID_FIELD_OVERRIDE*/M2, /*error:INVALID_METHOD_OVERRIDE_FROM_MIXIN*/M1 {}
with M2, /*error:INVALID_METHOD_OVERRIDE_FROM_MIXIN*/M1 {}
''');
}

Expand All @@ -2681,7 +2681,7 @@ class M2 {
class /*error:INCONSISTENT_METHOD_INHERITANCE*/T1 extends Base
with M1,
/*error:INVALID_METHOD_OVERRIDE_FROM_MIXIN,error:INVALID_FIELD_OVERRIDE*/M2 {}
/*error:INVALID_METHOD_OVERRIDE_FROM_MIXIN*/M2 {}
''');
}

Expand Down Expand Up @@ -3294,8 +3294,8 @@ class Base {
}
class GrandChild extends main.Child {
/*error:INVALID_FIELD_OVERRIDE*/var _f2;
/*error:INVALID_FIELD_OVERRIDE*/var _f3;
var _f2;
var _f3;
var _f4;
/*error:INVALID_METHOD_OVERRIDE*/String _m1() => null;
Expand All @@ -3306,7 +3306,7 @@ class GrandChild extends main.Child {
import 'helper.dart' as helper;
class Child extends helper.Base {
/*error:INVALID_FIELD_OVERRIDE*/var f1;
var f1;
var _f2;
var _f4;
Expand Down
6 changes: 3 additions & 3 deletions pkg/analyzer/test/src/task/strong/inferred_type_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3187,7 +3187,7 @@ main() {
test_inferParameterType_setter_fromField() async {
var mainUnit = await checkFileElement('''
class C extends D {
/*error:INVALID_FIELD_OVERRIDE*/set foo(x) {}
set foo(x) {}
}
class D {
int foo;
Expand Down Expand Up @@ -3601,7 +3601,7 @@ class A {
}
class B extends A {
/*error:INVALID_FIELD_OVERRIDE*/get x => 3;
get x => 3;
}
foo() {
Expand Down Expand Up @@ -3741,7 +3741,7 @@ class A<T> {
class B<E> extends A<E> {
E y;
/*error:INVALID_FIELD_OVERRIDE*/get x => y;
get x => y;
}
foo() {
Expand Down
Loading

0 comments on commit 1c504f8

Please sign in to comment.