From 93842ad8786bf29d9a06e805123f3a24ddbef778 Mon Sep 17 00:00:00 2001 From: Geoff Romer Date: Thu, 10 Mar 2022 16:46:43 -0800 Subject: [PATCH] Eliminate run-time errors from PatternMatch (#1126) --- executable_semantics/ast/pattern.cpp | 30 +++++++++++++++++++ executable_semantics/ast/pattern.h | 6 ++++ .../interpreter/interpreter.cpp | 12 ++------ .../interpreter/type_checker.cpp | 4 +++ .../basic_syntax/fail_nested_binding.carbon | 17 +++++++++++ 5 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 executable_semantics/testdata/basic_syntax/fail_nested_binding.carbon diff --git a/executable_semantics/ast/pattern.cpp b/executable_semantics/ast/pattern.cpp index bd4d51882121..fc3ca77c6149 100644 --- a/executable_semantics/ast/pattern.cpp +++ b/executable_semantics/ast/pattern.cpp @@ -51,6 +51,36 @@ void Pattern::Print(llvm::raw_ostream& out) const { } } +// Equivalent to `GetBindings`, but stores its output in `bindings` instead of +// returning it. +static void GetBindingsImpl( + const Pattern& pattern, + std::vector>& bindings) { + switch (pattern.kind()) { + case PatternKind::BindingPattern: + bindings.push_back(&cast(pattern)); + return; + case PatternKind::TuplePattern: + for (const Pattern* field : cast(pattern).fields()) { + GetBindingsImpl(*field, bindings); + } + return; + case PatternKind::AlternativePattern: + GetBindingsImpl(cast(pattern).arguments(), bindings); + return; + case PatternKind::AutoPattern: + case PatternKind::ExpressionPattern: + return; + } +} + +auto GetBindings(const Pattern& pattern) + -> std::vector> { + std::vector> result; + GetBindingsImpl(pattern, result); + return result; +} + auto PatternFromParenContents(Nonnull arena, SourceLocation source_loc, const ParenContents& paren_contents) -> Nonnull { diff --git a/executable_semantics/ast/pattern.h b/executable_semantics/ast/pattern.h index 08c33166c830..46b75364cd58 100644 --- a/executable_semantics/ast/pattern.h +++ b/executable_semantics/ast/pattern.h @@ -84,6 +84,12 @@ class Pattern : public AstNode { std::optional> value_; }; +class BindingPattern; + +// Returns all `BindingPattern`s in the AST subtree rooted at `pattern`. +auto GetBindings(const Pattern& pattern) + -> std::vector>; + // A pattern consisting of the `auto` keyword. class AutoPattern : public Pattern { public: diff --git a/executable_semantics/interpreter/interpreter.cpp b/executable_semantics/interpreter/interpreter.cpp index 09aee46b31db..cfd49424c363 100644 --- a/executable_semantics/interpreter/interpreter.cpp +++ b/executable_semantics/interpreter/interpreter.cpp @@ -178,11 +178,7 @@ auto PatternMatch(Nonnull p, Nonnull v, std::optional> bindings) -> bool { switch (p->kind()) { case Value::Kind::BindingPlaceholderValue: { - if (!bindings.has_value()) { - // TODO: move this to typechecker. - FATAL_COMPILATION_ERROR(source_loc) - << "Name bindings are not supported in this context"; - } + CHECK(bindings.has_value()); const auto& placeholder = cast(*p); if (placeholder.value_node().has_value()) { (*bindings)->Initialize(*placeholder.value_node(), v); @@ -194,11 +190,7 @@ auto PatternMatch(Nonnull p, Nonnull v, case Value::Kind::TupleValue: { const auto& p_tup = cast(*p); const auto& v_tup = cast(*v); - if (p_tup.elements().size() != v_tup.elements().size()) { - FATAL_PROGRAM_ERROR(source_loc) - << "arity mismatch in tuple pattern match:\n pattern: " - << p_tup << "\n value: " << v_tup; - } + CHECK(p_tup.elements().size() == v_tup.elements().size()); for (size_t i = 0; i < p_tup.elements().size(); ++i) { if (!PatternMatch(p_tup.elements()[i], v_tup.elements()[i], source_loc, bindings)) { diff --git a/executable_semantics/interpreter/type_checker.cpp b/executable_semantics/interpreter/type_checker.cpp index 5e43cc767236..8101884c5c35 100644 --- a/executable_semantics/interpreter/type_checker.cpp +++ b/executable_semantics/interpreter/type_checker.cpp @@ -840,6 +840,10 @@ void TypeChecker::TypeCheckPattern( } case PatternKind::BindingPattern: { auto& binding = cast(*p); + if (!GetBindings(binding.type()).empty()) { + FATAL_COMPILATION_ERROR(binding.type().source_loc()) + << "The type of a binding pattern cannot contain bindings."; + } TypeCheckPattern(&binding.type(), std::nullopt, impl_scope); Nonnull type = InterpPattern(&binding.type(), arena_, trace_); diff --git a/executable_semantics/testdata/basic_syntax/fail_nested_binding.carbon b/executable_semantics/testdata/basic_syntax/fail_nested_binding.carbon new file mode 100644 index 000000000000..8b233397bd0a --- /dev/null +++ b/executable_semantics/testdata/basic_syntax/fail_nested_binding.carbon @@ -0,0 +1,17 @@ +// 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 +// +// RUN: %{not} %{executable_semantics} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{executable_semantics} --trace %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{executable_semantics} %s +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/basic_syntax/fail_nested_binding.carbon:15: The type of a binding pattern cannot contain bindings. + +package ExecutableSemanticsTest api; + +fn Main() -> i32 { + var x: (T: Type) = 1; + return 1; +}