[fasta] Don't generate noSuchMethod forwarders in abstract classes
Bug: http://dartbug.com/33482
Change-Id: I4c9b2e9a43fdcd37238ede5814b2ad471f7b2c4d
Reviewed-on: https://dart-review.googlesource.com/60847
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Samir Jindel <sjindel@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
index ac59460..930a322 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
@@ -385,13 +385,10 @@
}
}
- // TODO(dmitryas): Find a better place for this routine.
- static bool hasUserDefinedNoSuchMethod(
- Class klass, ClassHierarchy hierarchy) {
+ bool hasUserDefinedNoSuchMethod(
+ Class klass, ClassHierarchy hierarchy, Class objectClass) {
Member noSuchMethod = hierarchy.getDispatchTarget(klass, noSuchMethodName);
- // `Object` doesn't have a superclass reference.
- return noSuchMethod != null &&
- noSuchMethod.enclosingClass.superclass != null;
+ return noSuchMethod != null && noSuchMethod.enclosingClass != objectClass;
}
void transformProcedureToNoSuchMethodForwarder(
@@ -439,57 +436,47 @@
/// class was modified.
bool addNoSuchMethodForwarders(
KernelTarget target, ClassHierarchy hierarchy) {
- if (!hasUserDefinedNoSuchMethod(cls, hierarchy)) {
+ if (cls.isAbstract ||
+ !hasUserDefinedNoSuchMethod(cls, hierarchy, target.objectClass)) {
return false;
}
Set<Name> existingForwardersNames = new Set<Name>();
Set<Name> existingSetterForwardersNames = new Set<Name>();
- if (cls.superclass != null &&
- hasUserDefinedNoSuchMethod(cls.superclass, hierarchy)) {
- List<Member> concrete = hierarchy.getDispatchTargets(cls.superclass);
- for (Member member in hierarchy.getInterfaceMembers(cls.superclass)) {
+ Class leastConcreteSuperclass = cls.superclass;
+ while (
+ leastConcreteSuperclass != null && leastConcreteSuperclass.isAbstract) {
+ leastConcreteSuperclass = leastConcreteSuperclass.superclass;
+ }
+ if (leastConcreteSuperclass != null &&
+ hasUserDefinedNoSuchMethod(
+ leastConcreteSuperclass, hierarchy, target.objectClass)) {
+ List<Member> concrete =
+ hierarchy.getDispatchTargets(leastConcreteSuperclass);
+ for (Member member
+ in hierarchy.getInterfaceMembers(leastConcreteSuperclass)) {
if (ClassHierarchy.findMemberByName(concrete, member.name) == null) {
existingForwardersNames.add(member.name);
}
}
List<Member> concreteSetters =
- hierarchy.getDispatchTargets(cls.superclass, setters: true);
- for (Member member
- in hierarchy.getInterfaceMembers(cls.superclass, setters: true)) {
+ hierarchy.getDispatchTargets(leastConcreteSuperclass, setters: true);
+ for (Member member in hierarchy
+ .getInterfaceMembers(leastConcreteSuperclass, setters: true)) {
if (ClassHierarchy.findMemberByName(concreteSetters, member.name) ==
null) {
existingSetterForwardersNames.add(member.name);
}
}
}
- if (cls.mixedInClass != null &&
- hasUserDefinedNoSuchMethod(cls.mixedInClass, hierarchy)) {
- List<Member> concrete = hierarchy.getDispatchTargets(cls.mixedInClass);
- for (Member member in hierarchy.getInterfaceMembers(cls.mixedInClass)) {
- if (ClassHierarchy.findMemberByName(concrete, member.name) == null) {
- existingForwardersNames.add(member.name);
- }
- }
-
- List<Member> concreteSetters =
- hierarchy.getDispatchTargets(cls.superclass, setters: true);
- for (Member member
- in hierarchy.getInterfaceMembers(cls.superclass, setters: true)) {
- if (ClassHierarchy.findMemberByName(concreteSetters, member.name) ==
- null) {
- existingSetterForwardersNames.add(member.name);
- }
- }
- }
-
- List<Member> concrete = hierarchy.getDispatchTargets(cls);
- List<Member> declared = hierarchy.getDeclaredMembers(cls);
Member noSuchMethod = ClassHierarchy.findMemberByName(
hierarchy.getInterfaceMembers(cls), noSuchMethodName);
+ List<Member> concrete = hierarchy.getDispatchTargets(cls);
+ List<Member> declared = hierarchy.getDeclaredMembers(cls);
+
bool changed = false;
for (Member member in hierarchy.getInterfaceMembers(cls)) {
if (member is Procedure &&
diff --git a/pkg/front_end/testcases/compile.status b/pkg/front_end/testcases/compile.status
index 1623e1d..c77eb3f 100644
--- a/pkg/front_end/testcases/compile.status
+++ b/pkg/front_end/testcases/compile.status
@@ -125,3 +125,8 @@
co19_language_metadata_syntax_t04: RuntimeError # Fasta doesn't recover well
external_import: RuntimeError # Expected -- test uses import which doesn't exist.
+
+no_such_method_forwarders/forwarders_not_assumed_from_mixin: Fail
+no_such_method_forwarders/no_forwarders_for_abstract_classes: Fail
+no_such_method_forwarders/no_forwarders_for_abstract_classes_chain: Fail
+no_such_method_forwarders/concrete_method_over_forwarder_in_mixin_application: Fail
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/concrete_method_over_forwarder_in_mixin_application.dart b/pkg/front_end/testcases/no_such_method_forwarders/concrete_method_over_forwarder_in_mixin_application.dart
new file mode 100644
index 0000000..de97201
--- /dev/null
+++ b/pkg/front_end/testcases/no_such_method_forwarders/concrete_method_over_forwarder_in_mixin_application.dart
@@ -0,0 +1,23 @@
+// Copyright (c) 2018, 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.
+
+// This test checks that when both LHS and RHS in a mixin application have a
+// concrete implementation of a method and a noSuchMethod forwarder for it, the
+// concrete implementation stays.
+
+abstract class I {
+ foo();
+}
+
+class A {
+ foo() {}
+}
+
+class B implements I {
+ noSuchMethod(_) => null;
+}
+
+class C extends A with B {}
+
+main() {}
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart b/pkg/front_end/testcases/no_such_method_forwarders/forwarders_not_assumed_from_mixin.dart
similarity index 66%
rename from pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart
rename to pkg/front_end/testcases/no_such_method_forwarders/forwarders_not_assumed_from_mixin.dart
index fcbc2e3..6a76ef8 100644
--- a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart
+++ b/pkg/front_end/testcases/no_such_method_forwarders/forwarders_not_assumed_from_mixin.dart
@@ -2,8 +2,9 @@
// 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.
-// This test checks that the noSuchMethod forwarder is not duplicated in classes
-// that mix in the classes that already have the forwarder.
+// This test checks that the noSuchMethod forwarder is not assumed to be mixed
+// in and is generated for classes that mix in a user-defined noSuchMethod and
+// have an abstract method.
abstract class I {
void foo();
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.direct.expect b/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.direct.expect
deleted file mode 100644
index 0283bae..0000000
--- a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.direct.expect
+++ /dev/null
@@ -1,27 +0,0 @@
-library;
-import self as self;
-import "dart:core" as core;
-
-abstract class I extends core::Object {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- abstract method foo() → void;
-}
-class A extends core::Object implements self::I {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- method noSuchMethod(core::Invocation i) → dynamic
- return null;
- no-such-method-forwarder method foo() → void
- return this.{self::A::noSuchMethod}(new core::_InvocationMirror::_withoutType("foo", const <core::Type>[], const <dynamic>[], core::Map::unmodifiable<core::Symbol, dynamic>(const <core::Symbol, dynamic>{}), false));
-}
-abstract class _B&Object&A = core::Object with self::A {
-}
-class B extends self::_B&Object&A {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
-}
-static method main() → dynamic {}
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.direct.transformed.expect b/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.direct.transformed.expect
deleted file mode 100644
index bf046d8..0000000
--- a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.direct.transformed.expect
+++ /dev/null
@@ -1,34 +0,0 @@
-library;
-import self as self;
-import "dart:core" as core;
-
-abstract class I extends core::Object {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- abstract method foo() → void;
-}
-class A extends core::Object implements self::I {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- method noSuchMethod(core::Invocation i) → dynamic
- return null;
- no-such-method-forwarder method foo() → void
- return this.{self::A::noSuchMethod}(new core::_InvocationMirror::_withoutType("foo", const <core::Type>[], const <dynamic>[], core::Map::unmodifiable<core::Symbol, dynamic>(const <core::Symbol, dynamic>{}), false));
-}
-abstract class _B&Object&A extends core::Object implements self::A {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- method noSuchMethod(core::Invocation i) → dynamic
- return null;
- no-such-method-forwarder method foo() → void
- return this.{self::A::noSuchMethod}(new core::_InvocationMirror::_withoutType("foo", const <core::Type>[], const <dynamic>[], core::Map::unmodifiable<core::Symbol, dynamic>(const <core::Symbol, dynamic>{}), false));
-}
-class B extends self::_B&Object&A {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
-}
-static method main() → dynamic {}
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.outline.expect b/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.outline.expect
deleted file mode 100644
index 63d5358..0000000
--- a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.outline.expect
+++ /dev/null
@@ -1,25 +0,0 @@
-library;
-import self as self;
-import "dart:core" as core;
-
-abstract class I extends core::Object {
- synthetic constructor •() → void
- ;
- abstract method foo() → void;
-}
-class A extends core::Object implements self::I {
- synthetic constructor •() → void
- ;
- method noSuchMethod(core::Invocation i) → dynamic
- ;
- no-such-method-forwarder method foo() → void
- return this.{self::A::noSuchMethod}(new core::_InvocationMirror::_withoutType("foo", const <core::Type>[], const <dynamic>[], core::Map::unmodifiable<core::Symbol, dynamic>(const <core::Symbol, dynamic>{}), false));
-}
-abstract class _B&Object&A = core::Object with self::A {
-}
-class B extends self::_B&Object&A {
- synthetic constructor •() → void
- ;
-}
-static method main() → dynamic
- ;
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.strong.expect b/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.strong.expect
deleted file mode 100644
index 0283bae..0000000
--- a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.strong.expect
+++ /dev/null
@@ -1,27 +0,0 @@
-library;
-import self as self;
-import "dart:core" as core;
-
-abstract class I extends core::Object {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- abstract method foo() → void;
-}
-class A extends core::Object implements self::I {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- method noSuchMethod(core::Invocation i) → dynamic
- return null;
- no-such-method-forwarder method foo() → void
- return this.{self::A::noSuchMethod}(new core::_InvocationMirror::_withoutType("foo", const <core::Type>[], const <dynamic>[], core::Map::unmodifiable<core::Symbol, dynamic>(const <core::Symbol, dynamic>{}), false));
-}
-abstract class _B&Object&A = core::Object with self::A {
-}
-class B extends self::_B&Object&A {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
-}
-static method main() → dynamic {}
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.strong.transformed.expect b/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.strong.transformed.expect
deleted file mode 100644
index bf046d8..0000000
--- a/pkg/front_end/testcases/no_such_method_forwarders/no_dup_from_mixin.dart.strong.transformed.expect
+++ /dev/null
@@ -1,34 +0,0 @@
-library;
-import self as self;
-import "dart:core" as core;
-
-abstract class I extends core::Object {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- abstract method foo() → void;
-}
-class A extends core::Object implements self::I {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- method noSuchMethod(core::Invocation i) → dynamic
- return null;
- no-such-method-forwarder method foo() → void
- return this.{self::A::noSuchMethod}(new core::_InvocationMirror::_withoutType("foo", const <core::Type>[], const <dynamic>[], core::Map::unmodifiable<core::Symbol, dynamic>(const <core::Symbol, dynamic>{}), false));
-}
-abstract class _B&Object&A extends core::Object implements self::A {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
- method noSuchMethod(core::Invocation i) → dynamic
- return null;
- no-such-method-forwarder method foo() → void
- return this.{self::A::noSuchMethod}(new core::_InvocationMirror::_withoutType("foo", const <core::Type>[], const <dynamic>[], core::Map::unmodifiable<core::Symbol, dynamic>(const <core::Symbol, dynamic>{}), false));
-}
-class B extends self::_B&Object&A {
- synthetic constructor •() → void
- : super core::Object::•()
- ;
-}
-static method main() → dynamic {}
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/no_forwarders_for_abstract_classes.dart b/pkg/front_end/testcases/no_such_method_forwarders/no_forwarders_for_abstract_classes.dart
new file mode 100644
index 0000000..bb14b72
--- /dev/null
+++ b/pkg/front_end/testcases/no_such_method_forwarders/no_forwarders_for_abstract_classes.dart
@@ -0,0 +1,20 @@
+// Copyright (c) 2018, 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.
+
+// This test checks that the noSuchMethod forwarders aren't generated for the
+// abstract classes that have user-defined noSuchMethod, but rather in their
+// concrete descendants.
+
+abstract class A {
+ noSuchMethod(i) => null;
+
+ // The forwarder for [foo] shouldn't be generated here.
+ void foo();
+}
+
+class B extends A {
+ // The forwarder for [foo] should be generated here.
+}
+
+main() {}
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/no_forwarders_for_abstract_classes_chain.dart b/pkg/front_end/testcases/no_such_method_forwarders/no_forwarders_for_abstract_classes_chain.dart
new file mode 100644
index 0000000..3ea89bf
--- /dev/null
+++ b/pkg/front_end/testcases/no_such_method_forwarders/no_forwarders_for_abstract_classes_chain.dart
@@ -0,0 +1,29 @@
+// Copyright (c) 2018, 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.
+
+// This test checks that the noSuchMethod forwarders aren't generated for the
+// abstract classes that have user-defined noSuchMethod, but rather in their
+// top-most concrete descendants. The immediate abstract children should not
+// receive the forwarders.
+
+abstract class A {
+ noSuchMethod(i) => null;
+
+ // The forwarder for [foo] shouldn't be generated here.
+ void foo();
+}
+
+abstract class B extends A {
+ // [B] shouldn't receive a forwarder.
+}
+
+class C extends B {
+ // The forwarder for [foo] should be generated here.
+}
+
+class D extends C {
+ // [D] shouldn't receiver a forwarder.
+}
+
+main() {}
diff --git a/pkg/front_end/testcases/outline.status b/pkg/front_end/testcases/outline.status
index 9cdafa9..603651f 100644
--- a/pkg/front_end/testcases/outline.status
+++ b/pkg/front_end/testcases/outline.status
@@ -252,3 +252,8 @@
rasta/issue_000047: Fail
rasta/native_is_illegal: Pass # Issue 29763
rasta/type_with_parse_error: Fail
+
+no_such_method_forwarders/forwarders_not_assumed_from_mixin: Fail
+no_such_method_forwarders/no_forwarders_for_abstract_classes: Fail
+no_such_method_forwarders/no_forwarders_for_abstract_classes_chain: Fail
+no_such_method_forwarders/concrete_method_over_forwarder_in_mixin_application: Fail
diff --git a/pkg/front_end/testcases/strong.status b/pkg/front_end/testcases/strong.status
index b0858b9..0b285a6 100644
--- a/pkg/front_end/testcases/strong.status
+++ b/pkg/front_end/testcases/strong.status
@@ -221,3 +221,8 @@
co19_language_metadata_syntax_t04: RuntimeError # Fasta doesn't recover well
external_import: RuntimeError # The native extension to import doesn't exist. This is ok.
+
+no_such_method_forwarders/forwarders_not_assumed_from_mixin: Fail
+no_such_method_forwarders/no_forwarders_for_abstract_classes: Fail
+no_such_method_forwarders/no_forwarders_for_abstract_classes_chain: Fail
+no_such_method_forwarders/concrete_method_over_forwarder_in_mixin_application: Fail
diff --git a/tests/language_2/language_2_dartdevc.status b/tests/language_2/language_2_dartdevc.status
index 278d7f4..dccaa3a 100644
--- a/tests/language_2/language_2_dartdevc.status
+++ b/tests/language_2/language_2_dartdevc.status
@@ -553,6 +553,9 @@
string_supertype_checked_test: CompileTimeError # Issue 31616
super_bound_closure_test/none: CompileTimeError # Issue 31533
super_call4_test/01: MissingCompileTimeError
+super_operator_index6_test: CompileTimeError # Issue 33498
+super_operator_index7_test: CompileTimeError # Issue 33498
+super_operator_index8_test: CompileTimeError # Issue 33498
switch_bad_case_test/01: MissingCompileTimeError
switch_bad_case_test/02: MissingCompileTimeError
switch_case_test/00: MissingCompileTimeError
diff --git a/tests/language_2/language_2_kernel.status b/tests/language_2/language_2_kernel.status
index b0a22c7..6c4d31a 100644
--- a/tests/language_2/language_2_kernel.status
+++ b/tests/language_2/language_2_kernel.status
@@ -655,6 +655,9 @@
vm/type_vm_test/36: MissingRuntimeError
[ $compiler == dartk && $runtime == vm ]
+super_operator_index6_test: CompileTimeError # Issue 33498
+super_operator_index7_test: CompileTimeError # Issue 33498
+super_operator_index8_test: CompileTimeError # Issue 33498
type_alias_equality_test/02: RuntimeError # Issue 31359
type_alias_equality_test/03: RuntimeError # Issue 31359
type_alias_equality_test/04: RuntimeError # Issue 31359
@@ -1387,6 +1390,9 @@
string_split_test: CompileTimeError
string_supertype_checked_test: CompileTimeError
super_bound_closure_test/none: CompileTimeError
+super_operator_index6_test: CompileTimeError # Issue 33498
+super_operator_index7_test: CompileTimeError # Issue 33498
+super_operator_index8_test: CompileTimeError # Issue 33498
try_catch_test/01: MissingCompileTimeError
type_promotion_functions_test/02: CompileTimeError
type_promotion_functions_test/03: CompileTimeError
