From f1ddded3a18c8500c863204d5150e9b759df2e89 Mon Sep 17 00:00:00 2001 From: Aneesh Durg Date: Mon, 20 Nov 2023 19:32:00 -0600 Subject: [PATCH] [explorer] Add support for `abstract` virtual methods (#3411) Completing the work from #2761 by adding additional test cases. Closes #2512 --------- Co-authored-by: Maan2003 Co-authored-by: Aleksei Ermakov --- explorer/interpreter/type_checker.cpp | 37 +++++++++++-- .../testdata/class/abstract_method.carbon | 19 +++++++ .../abstract_method_base_instance.carbon | 54 +++++++++++++++++++ ...fail_abstract_method_abstract_class.carbon | 26 +++++++++ .../fail_abstract_method_base_class.carbon | 22 ++++++++ .../class/fail_abstract_method_body.carbon | 19 +++++++ ...=> fail_abstract_method_final_impl.carbon} | 13 ++--- ..._abstract_method_non_abstract_class.carbon | 19 +++++++ 8 files changed, 197 insertions(+), 12 deletions(-) create mode 100644 explorer/testdata/class/abstract_method.carbon create mode 100644 explorer/testdata/class/abstract_method_base_instance.carbon create mode 100644 explorer/testdata/class/fail_abstract_method_abstract_class.carbon create mode 100644 explorer/testdata/class/fail_abstract_method_base_class.carbon create mode 100644 explorer/testdata/class/fail_abstract_method_body.carbon rename explorer/testdata/class/{fail_abstract_method_not_supported.carbon => fail_abstract_method_final_impl.carbon} (50%) create mode 100644 explorer/testdata/class/fail_abstract_method_non_abstract_class.carbon diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp index ef057e9b0898..da09149cd843 100644 --- a/explorer/interpreter/type_checker.cpp +++ b/explorer/interpreter/type_checker.cpp @@ -5167,16 +5167,27 @@ auto TypeChecker::DeclareClassDeclaration(Nonnull class_decl, } CARBON_CHECK(!fun->name().is_qualified()) << "qualified function name not permitted in class scope"; + + if (fun->virt_override() == VirtualOverride::Abstract && + fun->body().has_value()) { + return ProgramError(fun->source_loc()) + << "Error declaring `" << fun->name() << "`" + << ": abstract method cannot have a body."; + } + bool has_vtable_entry = class_vtable.find(fun->name().inner_name()) != class_vtable.end(); // TODO: Implement complete declaration logic from // `/docs/design/classes.md#virtual-methods`. switch (fun->virt_override()) { case VirtualOverride::Abstract: - // Not supported yet. - return ProgramError(fun->source_loc()) - << "Error declaring `" << fun->name() << "`" - << ": `abstract` methods are not yet supported."; + if (class_decl->extensibility() != ClassExtensibility::Abstract) { + return ProgramError(fun->source_loc()) + << "Error declaring `" << fun->name() << "`" + << ": `abstract` methods are allowed only in abstract " + "classes."; + } + break; case VirtualOverride::None: case VirtualOverride::Virtual: if (has_vtable_entry) { @@ -5237,6 +5248,24 @@ auto TypeChecker::DeclareClassDeclaration(Nonnull class_decl, } } + if (class_decl->extensibility() != ClassExtensibility::Abstract) { + auto abstract_method_it = std::find_if( + class_vtable.begin(), class_vtable.end(), [](const auto& vt) { + const auto* const fun = vt.getValue().first; + return fun->is_method() && + fun->virt_override() == VirtualOverride::Abstract; + }); + + if (abstract_method_it != class_vtable.end()) { + auto fun_name = GetName(*abstract_method_it->getValue().first); + CARBON_CHECK(fun_name.has_value()); + return ProgramError(class_decl->source_loc()) + << "Error declaring `" << class_decl->name() << "`" + << ": non abstract class should implement abstract method `" + << *fun_name << "`."; + } + } + // For class declaration `class MyType(T:! type, U:! AnInterface)`, `Self` // should have the value `MyType(T, U)`. const auto* self_type = arena_->New( diff --git a/explorer/testdata/class/abstract_method.carbon b/explorer/testdata/class/abstract_method.carbon new file mode 100644 index 000000000000..e8ecc02ba64e --- /dev/null +++ b/explorer/testdata/class/abstract_method.carbon @@ -0,0 +1,19 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// RUN: %{explorer-run} +// RUN: %{explorer-run-trace} + +package ExplorerTest api; + +abstract class A { + abstract fn Foo[self: Self](); +} + +fn Main() -> i32 { + return 0; +} + +// CHECK:STDOUT: result: 0 diff --git a/explorer/testdata/class/abstract_method_base_instance.carbon b/explorer/testdata/class/abstract_method_base_instance.carbon new file mode 100644 index 000000000000..afc7b70ea433 --- /dev/null +++ b/explorer/testdata/class/abstract_method_base_instance.carbon @@ -0,0 +1,54 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// RUN: %{explorer-run} +// RUN: %{explorer-run-trace} + +package ExplorerTest api; + +abstract class A { + abstract fn Foo[self: Self](); +} + +base class B { + extend base: A; + impl fn Foo[self: Self]() { + Print("B::Foo called!"); + } +} + +class C { + extend base: A; + impl fn Foo[self: Self]() { + Print("C::Foo called!"); + } +} + +abstract class D { + extend base: A; +} + +class E { + extend base: D; + impl fn Foo[self: Self]() { + Print("E::Foo called!"); + } +} + + +fn Main() -> i32 { + var b: B = { .base = {} }; + b.Foo(); + var c: C = { .base = {} }; + c.Foo(); + var e: E = { .base = { .base = {} } }; + e.Foo(); + return 0; +} + +// CHECK:STDOUT: B::Foo called! +// CHECK:STDOUT: C::Foo called! +// CHECK:STDOUT: E::Foo called! +// CHECK:STDOUT: result: 0 diff --git a/explorer/testdata/class/fail_abstract_method_abstract_class.carbon b/explorer/testdata/class/fail_abstract_method_abstract_class.carbon new file mode 100644 index 000000000000..fe1377bac876 --- /dev/null +++ b/explorer/testdata/class/fail_abstract_method_abstract_class.carbon @@ -0,0 +1,26 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// RUN: %{not} %{explorer-run} +// RUN: %{not} %{explorer-run-trace} + +package ExplorerTest api; + +abstract class A { + abstract fn Foo[self: Self](); +} + +abstract class B { + extend base: A; +} + +class C { + extend base: B; +// CHECK:STDERR: COMPILATION ERROR: fail_abstract_method_abstract_class.carbon:[[@LINE+1]]: Error declaring `C`: non abstract class should implement abstract method `Foo`. +} + +fn Main() -> i32 { + return 0; +} diff --git a/explorer/testdata/class/fail_abstract_method_base_class.carbon b/explorer/testdata/class/fail_abstract_method_base_class.carbon new file mode 100644 index 000000000000..009d6507c755 --- /dev/null +++ b/explorer/testdata/class/fail_abstract_method_base_class.carbon @@ -0,0 +1,22 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// RUN: %{not} %{explorer-run} +// RUN: %{not} %{explorer-run-trace} + +package ExplorerTest api; + +abstract class A { + abstract fn Foo[self: Self](); +} + +base class B { + extend base: A; +// CHECK:STDERR: COMPILATION ERROR: fail_abstract_method_base_class.carbon:[[@LINE+1]]: Error declaring `B`: non abstract class should implement abstract method `Foo`. +} + +fn Main() -> i32 { + return 0; +} diff --git a/explorer/testdata/class/fail_abstract_method_body.carbon b/explorer/testdata/class/fail_abstract_method_body.carbon new file mode 100644 index 000000000000..e9c827d99f83 --- /dev/null +++ b/explorer/testdata/class/fail_abstract_method_body.carbon @@ -0,0 +1,19 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// RUN: %{not} %{explorer-run} +// RUN: %{not} %{explorer-run-trace} + +package ExplorerTest api; + +abstract class A { + // CHECK:STDERR: COMPILATION ERROR: fail_abstract_method_body.carbon:[[@LINE+1]]: Error declaring `Foo`: abstract method cannot have a body. + abstract fn Foo[self: Self]() {} +} + + +fn Main() -> i32 { + return 0; +} diff --git a/explorer/testdata/class/fail_abstract_method_not_supported.carbon b/explorer/testdata/class/fail_abstract_method_final_impl.carbon similarity index 50% rename from explorer/testdata/class/fail_abstract_method_not_supported.carbon rename to explorer/testdata/class/fail_abstract_method_final_impl.carbon index 10f1468b9377..e2d1e4046ad4 100644 --- a/explorer/testdata/class/fail_abstract_method_not_supported.carbon +++ b/explorer/testdata/class/fail_abstract_method_final_impl.carbon @@ -6,18 +6,15 @@ package ExplorerTest api; -base class C { +abstract class A { + abstract fn Foo[self: Self](); } -class D { - extend base: C; - abstract fn Foo[self:Self]() -> i32 { - return 1; - // CHECK:STDERR: COMPILATION ERROR: fail_abstract_method_not_supported.carbon:[[@LINE+1]]: Error declaring `Foo`: `abstract` methods are not yet supported. - } +class B { + extend base: A; +// CHECK:STDERR: COMPILATION ERROR: fail_abstract_method_final_impl.carbon:[[@LINE+1]]: Error declaring `B`: non abstract class should implement abstract method `Foo`. } fn Main() -> i32 { - let d: D = {}; return 0; } diff --git a/explorer/testdata/class/fail_abstract_method_non_abstract_class.carbon b/explorer/testdata/class/fail_abstract_method_non_abstract_class.carbon new file mode 100644 index 000000000000..c3e1862a49b3 --- /dev/null +++ b/explorer/testdata/class/fail_abstract_method_non_abstract_class.carbon @@ -0,0 +1,19 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// RUN: %{not} %{explorer-run} +// RUN: %{not} %{explorer-run-trace} + +package ExplorerTest api; + +class A { + // CHECK:STDERR: COMPILATION ERROR: fail_abstract_method_non_abstract_class.carbon:[[@LINE+1]]: Error declaring `Foo`: `abstract` methods are allowed only in abstract classes. + abstract fn Foo[self: Self](); +} + + +fn Main() -> i32 { + return 0; +}