languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-23 16:20:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix various inheritance bugs

Browse source 
Fields can be overridden by subclasses
A setter does not conflict with a method
Eliminate outdated tests

Fixes issue 6482
Review URL: https://codereview.chromium.org//11312095

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@14583 260f80e4-7a28-3924-810f-c04153c831b5
This commit is contained in:
hausner@google.com 2012-11-06 17:54:27 +00:00
parent 9ee9f1d9a6
commit 99c505e9b3
9 changed files with 130 additions and 182 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

82
runtime/vm/class_finalizer.cc
View file

@ -955,6 +955,8 @@ void ClassFinalizer::ResolveAndFinalizeSignature(const Class& cls,
}
// Check if an instance field or method of same name exists
// in any super class.
static RawClass* FindSuperOwnerOfInstanceMember(const Class& cls,
const String& name) {
Class& super_class = Class::Handle();
@ -962,7 +964,6 @@ static RawClass* FindSuperOwnerOfInstanceMember(const Class& cls,
Field& field = Field::Handle();
super_class = cls.SuperClass();
while (!super_class.IsNull()) {
// Check if an instance member of same name exists in any super class.
function = super_class.LookupFunction(name);
if (!function.IsNull() && !function.is_static()) {
return super_class.raw();
@ -977,13 +978,13 @@ static RawClass* FindSuperOwnerOfInstanceMember(const Class& cls,
}
// Check if an instance method of same name exists in any super class.
static RawClass* FindSuperOwnerOfFunction(const Class& cls,
const String& name) {
Class& super_class = Class::Handle();
Function& function = Function::Handle();
super_class = cls.SuperClass();
while (!super_class.IsNull()) {
// Check if a function of same name exists in any super class.
function = super_class.LookupFunction(name);
if (!function.IsNull() && !function.is_static()) {
return super_class.raw();
@ -1023,8 +1024,6 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
// The only compile errors we report are therefore:
// - a getter having the same name as a method (but not a getter) in a super
// class or in a subclass.
// - a setter having the same name as a method (but not a setter) in a super
// class or in a subclass.
// - a static field, instance field, or static method (but not an instance
// method) having the same name as an instance member in a super class.
@ -1042,18 +1041,36 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
type = FinalizeType(cls, type, kCanonicalize);
field.set_type(type);
name = field.name();
super_class = FindSuperOwnerOfInstanceMember(cls, name);
if (!super_class.IsNull()) {
const String& class_name = String::Handle(cls.Name());
const String& super_class_name = String::Handle(super_class.Name());
const Script& script = Script::Handle(cls.script());
ReportError(script, field.token_pos(),
"field '%s' of class '%s' conflicts with instance "
"member '%s' of super class '%s'",
name.ToCString(),
class_name.ToCString(),
name.ToCString(),
super_class_name.ToCString());
if (field.is_static()) {
super_class = FindSuperOwnerOfInstanceMember(cls, name);
if (!super_class.IsNull()) {
const String& class_name = String::Handle(cls.Name());
const String& super_class_name = String::Handle(super_class.Name());
const Script& script = Script::Handle(cls.script());
ReportError(script, field.token_pos(),
"static field '%s' of class '%s' conflicts with "
"instance member '%s' of super class '%s'",
name.ToCString(),
class_name.ToCString(),
name.ToCString(),
super_class_name.ToCString());
}
} else {
// Instance field. Check whether the field overrides a method
// (but not getter).
super_class = FindSuperOwnerOfFunction(cls, name);
if (!super_class.IsNull()) {
const String& class_name = String::Handle(cls.Name());
const String& super_class_name = String::Handle(super_class.Name());
const Script& script = Script::Handle(cls.script());
ReportError(script, field.token_pos(),
"field '%s' of class '%s' conflicts with method '%s' "
"of super class '%s'",
name.ToCString(),
class_name.ToCString(),
name.ToCString(),
super_class_name.ToCString());
}
}
}
// Collect interfaces, super interfaces, and super classes of this class.
@ -1131,22 +1148,9 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
name.ToCString(),
super_class_name.ToCString());
}
} else if (function.IsSetterFunction()) {
name = Field::NameFromSetter(function_name);
super_class = FindSuperOwnerOfFunction(cls, name);
if (!super_class.IsNull()) {
const String& class_name = String::Handle(cls.Name());
const String& super_class_name = String::Handle(super_class.Name());
const Script& script = Script::Handle(cls.script());
ReportError(script, function.token_pos(),
"setter '%s' of class '%s' conflicts with "
"function '%s' of super class '%s'",
name.ToCString(),
class_name.ToCString(),
name.ToCString(),
super_class_name.ToCString());
}
} else {
} else if (!function.IsSetterFunction()) {
// A function cannot conflict with a setter, since they cannot
// have the same name. Thus, we do not need to check setters.
name = Field::GetterName(function_name);
super_class = FindSuperOwnerOfFunction(cls, name);
if (!super_class.IsNull()) {
@ -1161,20 +1165,6 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
function_name.ToCString(),
super_class_name.ToCString());
}
name = Field::SetterName(function_name);
super_class = FindSuperOwnerOfFunction(cls, name);
if (!super_class.IsNull()) {
const String& class_name = String::Handle(cls.Name());
const String& super_class_name = String::Handle(super_class.Name());
const Script& script = Script::Handle(cls.script());
ReportError(script, function.token_pos(),
"function '%s' of class '%s' conflicts with "
"setter '%s' of super class '%s'",
function_name.ToCString(),
class_name.ToCString(),
function_name.ToCString(),
super_class_name.ToCString());
}
}
}
}

120
runtime/vm/parser.cc
View file

@ -584,46 +584,63 @@ class ClassDesc : public ValueObject {
bool FunctionNameExists(const String& name, RawFunction::Kind kind) const {
// First check if a function or field of same name exists.
if (NameExists<Function>(functions_, name) ||
NameExists<Field>(fields_, name)) {
if (FunctionExists(name)) {
return true;
}
String& accessor_name = String::Handle();
if (kind != RawFunction::kSetterFunction) {
// Check if a getter function of same name exists.
accessor_name = Field::GetterName(name);
if (NameExists<Function>(functions_, accessor_name)) {
// Now check whether there is a field and whether its implicit getter
// or setter collides with the name.
Field* field = LookupField(name);
if (field != NULL) {
if (kind == RawFunction::kSetterFunction) {
// It's ok to have an implicit getter, it does not collide with
// this setter function.
if (!field->is_final()) {
return true;
}
} else {
// The implicit getter of the field collides with the name.
return true;
}
}
if (kind != RawFunction::kGetterFunction) {
String& accessor_name = String::Handle();
if (kind == RawFunction::kSetterFunction) {
// Check if a setter function of same name exists.
accessor_name = Field::SetterName(name);
if (NameExists<Function>(functions_, accessor_name)) {
if (FunctionExists(accessor_name)) {
return true;
}
} else {
// Check if a getter function of same name exists.
accessor_name = Field::GetterName(name);
if (FunctionExists(accessor_name)) {
return true;
}
}
return false;
}
bool FieldNameExists(const String& name) const {
bool FieldNameExists(const String& name, bool check_setter) const {
// First check if a function or field of same name exists.
if (NameExists<Function>(functions_, name) ||
NameExists<Field>(fields_, name)) {
if (FunctionExists(name) || FieldExists(name)) {
return true;
}
// Now check if a getter/setter function of same name exists.
String& getter_name = String::Handle(Field::GetterName(name));
String& setter_name = String::Handle(Field::SetterName(name));
if (NameExists<Function>(functions_, getter_name) ||
NameExists<Function>(functions_, setter_name)) {
if (FunctionExists(getter_name)) {
return true;
}
if (check_setter) {
String& setter_name = String::Handle(Field::SetterName(name));
if (FunctionExists(setter_name)) {
return true;
}
}
return false;
}
void AddFunction(const Function& function) {
ASSERT(!NameExists<Function>(functions_, String::Handle(function.name())));
ASSERT(!FunctionExists(String::Handle(function.name())));
functions_.Add(function);
}
@ -632,7 +649,7 @@ class ClassDesc : public ValueObject {
}
void AddField(const Field& field) {
ASSERT(!NameExists<Field>(fields_, String::Handle(field.name())));
ASSERT(!FieldExists(String::Handle(field.name())));
fields_.Add(field);
}
@ -685,18 +702,38 @@ class ClassDesc : public ValueObject {
}
private:
template<typename T>
bool NameExists(const GrowableObjectArray& list, const String& name) const {
Field* LookupField(const String& name) const {
String& test_name = String::Handle();
T& obj = T::Handle();
for (int i = 0; i < list.Length(); i++) {
obj ^= list.At(i);
test_name = obj.name();
Field& field = Field::Handle();
for (int i = 0; i < fields_.Length(); i++) {
field ^= fields_.At(i);
test_name = field.name();
if (name.Equals(test_name)) {
return true;
return &field;
}
}
return false;
return NULL;
}
bool FieldExists(const String& name) const {
return LookupField(name) != NULL;
}
Function* LookupFunction(const String& name) const {
String& test_name = String::Handle();
Function& func = Function::Handle();
for (int i = 0; i < functions_.Length(); i++) {
func ^= functions_.At(i);
test_name = func.name();
if (name.Equals(test_name)) {
return &func;
}
}
return NULL;
}
bool FunctionExists(const String& name) const {
return LookupFunction(name) != NULL;
}
const Class& clazz_;
@ -2738,6 +2775,7 @@ void Parser::ParseFieldDefinition(ClassDesc* members, MemberDesc* field) {
ASSERT(field->type != NULL);
ASSERT(field->name_pos > 0);
ASSERT(current_member_ == field);
// All const fields are also final.
ASSERT(!field->has_const || field->has_final);
if (field->has_abstract) {
@ -2749,7 +2787,7 @@ void Parser::ParseFieldDefinition(ClassDesc* members, MemberDesc* field) {
if (field->has_factory) {
ErrorMsg("keyword 'factory' not allowed in field declaration");
}
if (members->FieldNameExists(*field->name)) {
if (members->FieldNameExists(*field->name, !field->has_final)) {
ErrorMsg(field->name_pos,
"'%s' field/method already defined\n", field->name->ToCString());
}
@ -3866,10 +3904,14 @@ void Parser::ParseTopLevelVariable(TopLevel* top_level) {
ErrorMsg(name_pos, "getter for '%s' is already defined",
var_name.ToCString());
}
accessor_name = Field::SetterName(var_name);
if (library_.LookupLocalObject(accessor_name) != Object::null()) {
ErrorMsg(name_pos, "setter for '%s' is already defined",
var_name.ToCString());
// A const or final variable does not define an implicit setter,
// so we only check setters for non-final variables.
if (!is_final) {
accessor_name = Field::SetterName(var_name);
if (library_.LookupLocalObject(accessor_name) != Object::null()) {
ErrorMsg(name_pos, "setter for '%s' is already defined",
var_name.ToCString());
}
}
field = Field::New(var_name, is_static, is_final, is_const,
@ -3957,11 +3999,8 @@ void Parser::ParseTopLevelFunction(TopLevel* top_level) {
ErrorMsg(name_pos, "'%s' is already defined as getter",
func_name.ToCString());
}
accessor_name = Field::SetterName(func_name);
if (library_.LookupLocalObject(accessor_name) != Object::null()) {
ErrorMsg(name_pos, "'%s' is already defined as setter",
func_name.ToCString());
}
// A setter named x= may co-exist with a function named x, thus we do
// not need to check setters.
if (CurrentToken() != Token::kLPAREN) {
ErrorMsg("'(' expected");
@ -4071,10 +4110,19 @@ void Parser::ParseTopLevelAccessor(TopLevel* top_level) {
is_getter ? "getter" : "setter");
}
if (library_.LookupLocalObject(*field_name) != Object::null()) {
if (is_getter && library_.LookupLocalObject(*field_name) != Object::null()) {
ErrorMsg(name_pos, "'%s' is already defined in this library",
field_name->ToCString());
}
if (!is_getter) {
// Check whether there is a field with the same name that has an implicit
// setter.
const Field& field = Field::Handle(library_.LookupLocalField(*field_name));
if (!field.IsNull() && !field.is_final()) {
ErrorMsg(name_pos, "Variable '%s' is already defined in this library",
field_name->ToCString());
}
}
bool found = library_.LookupLocalObject(accessor_name) != Object::null();
if (found && !is_patch) {
ErrorMsg(name_pos, "%s for '%s' is already defined",

16
tests/co19/co19-runtime.status
View file

@ -35,16 +35,12 @@ Language/05_Variables/05_Variables_A05_t14: Fail # TODO(vm-team): Please triage
Language/05_Variables/05_Variables_A05_t15: Fail # TODO(vm-team): Please triage this failure.
Language/05_Variables/1_Evaluation_of_Implicit_Variable_Getters_A01_t02: Fail # TODO(vm-team): Please triage this failure.
Language/05_Variables/1_Evaluation_of_Implicit_Variable_Getters_A01_t05: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/2_Getters_A01_t03: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/2_Getters_A01_t05: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/3_Setters_A04_t01: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/3_Setters_A04_t02: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/3_Setters_A04_t03: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/3_Setters_A04_t04: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/3_Setters_A04_t05: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/3_Setters_A04_t06: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/3_Setters_A04_t07: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/3_Setters_A04_t08: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/2_Getters_A01_t03: Fail # Expects compile-time error when getter has empty parameter list.
Language/07_Classes/2_Getters_A01_t05: Fail # Expects compile-time error when getter has empty parameter list.
Language/07_Classes/3_Setters_A04_t03: Fail, OK # Syntax error, issue 304
Language/07_Classes/3_Setters_A04_t06: Fail, OK # Syntax error, issue 304
Language/07_Classes/3_Setters_A04_t07: Fail, OK # test error, issue 305
Language/07_Classes/3_Setters_A04_t08: Fail, OK # test error, issue 305
Language/07_Classes/4_Abstract_Instance_Members_A03_t02: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/4_Abstract_Instance_Members_A03_t03: Fail # TODO(vm-team): Please triage this failure.
Language/07_Classes/4_Abstract_Instance_Members_A03_t04: Fail # TODO(vm-team): Please triage this failure.

23
tests/language/field7a_negative_test.dart
View file

@ -1,23 +0,0 @@
// Copyright (c) 2011, 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.
// Dart test to catch error reporting bugs in class fields declarations.
// Should be an error because we have a function overriding a setter name.
class A {
void set a(var val) {
int i = val;
}
int a() {
return 1;
}
}
class Field7aNegativeTest {
static testMain() {
}
}
main() {
Field7aNegativeTest.testMain();
}

14
tests/language/language.status
View file

@ -19,9 +19,6 @@
part_test: Fail
part2_test: Fail
gc_test: Fail # Issue 1487
field_override_test/none: Fail # Issue 742: field shadowing now allowed
field_override_test/01: Fail # Issue 742: field shadowing now allowed
super_field_access_test: Fail # Issue 742: field shadowing now allowed
pseudo_kw_illegal_test/14: Fail # Issue 356
# These bugs refer currently ongoing language discussions.
@ -69,7 +66,7 @@ gc_test: Skip # Issue 1487, flaky.
gc_test: Skip # Takes too long.
[ $compiler == none && $unchecked ]
field_override_test/02: Fail # Issue 742: field shadowing now allowed
# Only checked mode reports an error on type assignment
# problems in compile time constants.
@ -139,7 +136,6 @@ disable_privacy_test: Fail # Issue 1882: Needs --disable_privacy support.
factory5_test/00: Fail # issue 3079
field_method4_negative_test: Fail # Runtime only test, rewrite as multitest
field7_negative_test: Fail, OK # language changed, test issue 5249
field7a_negative_test: Fail, OK # language changed, test issue 5249
first_class_types_literals_test: Fail # issue 6283
getter_no_setter_test/01: Fail # Fails to detect compile-time error.
getter_no_setter2_test/01: Fail # Fails to detect compile-time error.
@ -157,8 +153,6 @@ named_parameters_with_object_property_names_test: Fail # Issue 2137
many_overridden_no_such_method_test: Fail
no_such_method_negative_test: Fail # Runtime only test, rewrite as multitest
override_field_test/03: Fail # still a valid test? Issue 3894
override_field_test/04: Fail # still a valid test? Issue 3656
override_field_method3_negative_test: Fail, OK # test issue 5249
override_field_method6_negative_test: Fail, OK # test issue 5249
overridden_no_such_method_test: Fail
parameter_initializer6_negative_test: Fail # language change 4288
@ -457,7 +451,6 @@ named_parameters_aggregated_test/01: Fail # http://dartbug.com/5519
named_parameters_aggregated_test/03: Fail # http://dartbug.com/5519
named_parameters_aggregated_test/04: Fail # http://dartbug.com/5519
not_enough_positional_arguments_test/01: Fail # http://dartbug.com/5519
override_field_test/04: Fail # http://dartbug.com/5519
static_field3_test/01: Fail # http://dartbug.com/5519
static_field3_test/02: Fail # http://dartbug.com/5519
static_field3_test/03: Fail # http://dartbug.com/5519
@ -501,7 +494,6 @@ field2_negative_test: Fail
field4_negative_test: Fail
field5_negative_test: Fail
field6a_negative_test: Fail
field7a_negative_test: Fail
interface_factory_constructor_negative_test: Fail
interface_static_method_negative_test: Fail
non_const_super_negative_test: Fail
@ -543,11 +535,7 @@ syntax_test/47: Fail
constructor5_test: Fail
constructor6_test: Fail
closure_in_initializer_test: Fail
field_override_test/01: Fail
field_override_test/02: Fail
field_override_test/none: Fail
gc_test: Fail
super_field_access_test: Fail
super_first_constructor_test: Fail
# VM specific tests.
disable_privacy_test: Fail, Ok

2
tests/language/language_dart2js.status
View file

@ -261,7 +261,6 @@ field3_negative_test: Fail # Negative language test.
field4_negative_test: Fail # Negative language test.
field5_negative_test: Fail # Negative language test.
field6a_negative_test: Fail # Negative language test.
field7a_negative_test: Fail # Negative language test.
final_for_in_variable_test/01: Fail # Negative language test
final_param_negative_test: Fail # Negative language test.
final_var_negative_test: Fail # Negative language test.
@ -276,7 +275,6 @@ map_literal2_negative_test: Fail # Negative language test.
non_const_super_negative_test: Fail # Negative language test.
number_identifier_negative_test: Fail # Negative language test.
operator1_negative_test: Fail # Negative language test.
override_field_test/04: Fail # Broken test.
prefix16_test: Fail
prefix18_negative_test: Fail # Negative language test.
prefix20_negative_test: Fail # Negative language test.

22
tests/language/override_field_method3_negative_test.dart
View file

@ -1,22 +0,0 @@
// Copyright (c) 2011, 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.
// Dart test error for overriding setter with method.
class A {
set foo(x) { }
}
class B extends A {
foo(x) {} // method cannot override setter.
}
class OverrideFieldMethod3NegativeTest {
static testMain() {
new B().foo(10);
}
}
main() {
OverrideFieldMethod3NegativeTest.testMain();
}

22
tests/language/override_field_method6_negative_test.dart
View file

@ -1,22 +0,0 @@
// Copyright (c) 2011, 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.
// Dart test error for overriding method with setter.
class A {
foo(x) { }
}
class B extends A {
set foo(x) { } // setter cannot override method.
}
class OverrideFieldMethod6NegativeTest {
static testMain() {
new B().foo = 10;
}
}
main() {
OverrideFieldMethod6NegativeTest.testMain();
}

11
tests/language/override_field_test.dart
View file

@ -4,19 +4,14 @@
// Dart test checking that static/instance field shadowing do not conflict.
class A {
A() {} // DartC has no implicit constructors yet.
int instanceFieldInA;
static int staticFieldInA;
}
class B extends A {
B() : super() {} // DartC has no implicit constructors yet.
static int instanceFieldInA; /// 01: compile-time error
int staticFieldInA; /// 02: static type warning
static int staticFieldInA; /// 03: static type warning
int instanceFieldInA; /// 04: compile-time error
static int instanceFieldInA; /// 01: compile-time error
int staticFieldInA; /// 02: static type warning
static int staticFieldInA; /// 03: static type warning
}
main() {