From 7fe06a5d2f6311863db401809969f06e3143d1de Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 22 Feb 2023 13:51:25 -0800 Subject: [PATCH] Unify handling of calls to functions and to methods. (#2620) This fixes some bugs in each, where the fixes had only been made on one side of the switch or the other. Also don't forget to instantiate deduced generic arguments in a call when we read them out of the AST. --- explorer/interpreter/interpreter.cpp | 109 ++++++++---------- .../fail_call_method_before_typecheck.carbon | 21 ++++ .../class/fail_call_undefined_method.carbon | 19 +++ .../instantiate_deduced.carbon | 22 ++++ 4 files changed, 110 insertions(+), 61 deletions(-) create mode 100644 explorer/testdata/class/fail_call_method_before_typecheck.carbon create mode 100644 explorer/testdata/class/fail_call_undefined_method.carbon create mode 100644 explorer/testdata/generic_function/instantiate_deduced.carbon diff --git a/explorer/interpreter/interpreter.cpp b/explorer/interpreter/interpreter.cpp index 5c87ba4e03b1..09637c59cfbc 100644 --- a/explorer/interpreter/interpreter.cpp +++ b/explorer/interpreter/interpreter.cpp @@ -964,9 +964,13 @@ auto Interpreter::CallFunction(const CallExpression& call, return todo_.FinishAction(arena_->New( &alt.choice(), &alt.alternative(), cast(arg))); } - case Value::Kind::FunctionValue: { - const auto& fun_val = cast(*fun); - const FunctionDeclaration& function = fun_val.declaration(); + case Value::Kind::FunctionValue: + case Value::Kind::BoundMethodValue: { + const auto* func_val = dyn_cast(fun); + const auto* method_val = dyn_cast(fun); + + const FunctionDeclaration& function = + func_val ? func_val->declaration() : method_val->declaration(); if (!function.body().has_value()) { return ProgramError(call.source_loc()) << "attempt to call function `" << function.name() @@ -977,25 +981,35 @@ auto Interpreter::CallFunction(const CallExpression& call, << "attempt to call function `" << function.name() << "` that has not been fully type-checked"; } + RuntimeScope binding_scope(&heap_); - // Bring the class type arguments into scope. - for (const auto& [bind, val] : fun_val.type_args()) { - binding_scope.Initialize(bind, val); - } - // Bring the deduced type arguments into scope. + + // Bring the deduced arguments and their witnesses into scope. for (const auto& [bind, val] : call.deduced_args()) { - binding_scope.Initialize(bind, val); + CARBON_ASSIGN_OR_RETURN(Nonnull inst_val, + InstantiateType(val, call.source_loc())); + binding_scope.Initialize(bind->original(), inst_val); } - // Bring the impl witness tables into scope. for (const auto& [impl_bind, witness] : witnesses) { - binding_scope.Initialize(impl_bind, witness); + binding_scope.Initialize(impl_bind->original(), witness); } - for (const auto& [impl_bind, witness] : fun_val.witnesses()) { - binding_scope.Initialize(impl_bind, witness); + + // Bring the arguments that are determined by the function value into + // scope. This includes the arguments for the class of which the function + // is a member. + for (const auto& [bind, val] : + func_val ? func_val->type_args() : method_val->type_args()) { + binding_scope.Initialize(bind->original(), val); } + for (const auto& [impl_bind, witness] : + func_val ? func_val->witnesses() : method_val->witnesses()) { + binding_scope.Initialize(impl_bind->original(), witness); + } + // Enter the binding scope to make any deduced arguments visible before - // we resolve the parameter type. + // we resolve the self type and parameter type. todo_.CurrentAction().StartScope(std::move(binding_scope)); + CARBON_ASSIGN_OR_RETURN( Nonnull converted_args, Convert(arg, &function.param_pattern().static_type(), @@ -1003,59 +1017,32 @@ auto Interpreter::CallFunction(const CallExpression& call, RuntimeScope function_scope(&heap_); BindingMap generic_args; + + // Bind the receiver to the `self` parameter, if there is one. + if (method_val) { + CARBON_CHECK(function.is_method()); + const auto* self_pattern = &function.self_pattern().value(); + if (const auto* placeholder = + dyn_cast(self_pattern)) { + // TODO: move this logic into PatternMatch + if (placeholder->value_node().has_value()) { + function_scope.Bind(*placeholder->value_node(), + method_val->receiver()); + } + } else { + CARBON_CHECK(PatternMatch(self_pattern, method_val->receiver(), + call.source_loc(), &function_scope, + generic_args, trace_stream_, this->arena_)); + } + } + + // Bind the arguments to the parameters. CARBON_CHECK(PatternMatch( &function.param_pattern().value(), converted_args, call.source_loc(), &function_scope, generic_args, trace_stream_, this->arena_)); return todo_.Spawn(std::make_unique(*function.body()), std::move(function_scope)); } - case Value::Kind::BoundMethodValue: { - const auto& m = cast(*fun); - const FunctionDeclaration& method = m.declaration(); - CARBON_CHECK(method.is_method()); - CARBON_ASSIGN_OR_RETURN( - Nonnull converted_args, - Convert(arg, &method.param_pattern().static_type(), - call.source_loc())); - RuntimeScope method_scope(&heap_); - BindingMap generic_args; - // Bind the receiver to the `self` parameter. - const auto* p = &method.self_pattern().value(); - if (p->kind() == Value::Kind::BindingPlaceholderValue) { - // TODO: move this logic into PatternMatch - const auto& placeholder = cast(*p); - if (placeholder.value_node().has_value()) { - method_scope.Bind(*placeholder.value_node(), m.receiver()); - } - } else { - CARBON_CHECK(PatternMatch(&method.self_pattern().value(), m.receiver(), - call.source_loc(), &method_scope, - generic_args, trace_stream_, this->arena_)); - } - // Bind the arguments to the parameters. - CARBON_CHECK(PatternMatch(&method.param_pattern().value(), converted_args, - call.source_loc(), &method_scope, generic_args, - trace_stream_, this->arena_)); - // Bring the class type arguments into scope. - for (const auto& [bind, val] : m.type_args()) { - method_scope.Initialize(bind->original(), val); - } - // Bring the deduced type arguments into scope. - for (const auto& [bind, val] : call.deduced_args()) { - method_scope.Initialize(bind->original(), val); - } - // Bring the impl witness tables into scope. - for (const auto& [impl_bind, witness] : witnesses) { - method_scope.Initialize(impl_bind->original(), witness); - } - for (const auto& [impl_bind, witness] : m.witnesses()) { - method_scope.Initialize(impl_bind->original(), witness); - } - CARBON_CHECK(method.body().has_value()) - << "Calling a method that's missing a body"; - return todo_.Spawn(std::make_unique(*method.body()), - std::move(method_scope)); - } case Value::Kind::ParameterizedEntityName: { const auto& name = cast(*fun); const Declaration& decl = name.declaration(); diff --git a/explorer/testdata/class/fail_call_method_before_typecheck.carbon b/explorer/testdata/class/fail_call_method_before_typecheck.carbon new file mode 100644 index 000000000000..2d19c9bf7b98 --- /dev/null +++ b/explorer/testdata/class/fail_call_method_before_typecheck.carbon @@ -0,0 +1,21 @@ +// 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 { + fn F[self: Self]() -> type { + // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/class/fail_call_method_before_typecheck.carbon:[[@LINE+1]]: attempt to call function `F` that has not been fully type-checked + var a: ({} as A).F() = 0; + return i32; + } +} + +fn Main() -> i32 { + return 0; +} diff --git a/explorer/testdata/class/fail_call_undefined_method.carbon b/explorer/testdata/class/fail_call_undefined_method.carbon new file mode 100644 index 000000000000..ae421bd96217 --- /dev/null +++ b/explorer/testdata/class/fail_call_undefined_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: %{not} %{explorer-run} +// RUN: %{not} %{explorer-run-trace} + +package ExplorerTest api; + +class A { + fn F[self: Self]() -> i32; +} + +fn Main() -> i32 { + var a: A = {}; + // CHECK:STDERR: RUNTIME ERROR: {{.*}}/explorer/testdata/class/fail_call_undefined_method.carbon:[[@LINE+1]]: attempt to call function `F` that has not been defined + return a.F(); +} diff --git a/explorer/testdata/generic_function/instantiate_deduced.carbon b/explorer/testdata/generic_function/instantiate_deduced.carbon new file mode 100644 index 000000000000..cf36bddb5f0f --- /dev/null +++ b/explorer/testdata/generic_function/instantiate_deduced.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: %{explorer-run} +// RUN: %{explorer-run-trace} +// CHECK:STDOUT: result: 0 + +package ExplorerTest api; + +fn ReturnIndirectly[T:! type](direct: bool, x: T) -> type { + if (direct) { + return T; + } else { + return ReturnIndirectly(true, x); + } +} + +fn Main() -> ReturnIndirectly(false, 0) { + return 0; +}