From 721743bc58ba21cf9b960c559702664286d64fb1 Mon Sep 17 00:00:00 2001 From: Jon Meow <46229924+jonmeow@users.noreply.github.com> Date: Mon, 27 Sep 2021 09:05:22 -0700 Subject: [PATCH] Change Alternative to a class (#853) Revives BisonWrap because this seems a reasonable use of it (avoiding the need to have an std::optional or pointer for Alternative, both of which I thought could be unclear about the intent). --- executable_semantics/ast/declaration.cpp | 4 +- executable_semantics/ast/declaration.h | 24 +++++++--- .../interpreter/interpreter.cpp | 6 +-- .../interpreter/type_checker.cpp | 6 +-- executable_semantics/syntax/BUILD | 7 +++ executable_semantics/syntax/bison_wrap.h | 44 +++++++++++++++++++ executable_semantics/syntax/parser.ypp | 15 ++++--- 7 files changed, 84 insertions(+), 22 deletions(-) create mode 100644 executable_semantics/syntax/bison_wrap.h diff --git a/executable_semantics/ast/declaration.cpp b/executable_semantics/ast/declaration.cpp index 8477c3dab4d0..e90bf84231e5 100644 --- a/executable_semantics/ast/declaration.cpp +++ b/executable_semantics/ast/declaration.cpp @@ -30,8 +30,8 @@ void Declaration::Print(llvm::raw_ostream& out) const { case Kind::ChoiceDeclaration: { const auto& choice = cast(*this); out << "choice " << choice.Name() << " {\n"; - for (const auto& [name, signature] : choice.Alternatives()) { - out << "alt " << name << " " << *signature << ";\n"; + for (const auto& alt : choice.Alternatives()) { + out << "alt " << alt.name() << " " << alt.signature() << ";\n"; } out << "}\n"; break; diff --git a/executable_semantics/ast/declaration.h b/executable_semantics/ast/declaration.h index 90ab7d428bc9..9290b708ba9f 100644 --- a/executable_semantics/ast/declaration.h +++ b/executable_semantics/ast/declaration.h @@ -95,10 +95,21 @@ class ClassDeclaration : public Declaration { class ChoiceDeclaration : public Declaration { public: - ChoiceDeclaration( - SourceLocation loc, std::string name, - std::vector>> - alternatives) + class Alternative { + public: + Alternative(std::string name, Nonnull signature) + : name_(name), signature_(signature) {} + + auto name() const -> const std::string& { return name_; } + auto signature() const -> const Expression& { return *signature_; } + + private: + std::string name_; + Nonnull signature_; + }; + + ChoiceDeclaration(SourceLocation loc, std::string name, + std::vector alternatives) : Declaration(Kind::ChoiceDeclaration, loc), name(std::move(name)), alternatives(std::move(alternatives)) {} @@ -108,14 +119,13 @@ class ChoiceDeclaration : public Declaration { } auto Name() const -> const std::string& { return name; } - auto Alternatives() const -> const - std::vector>>& { + auto Alternatives() const -> const std::vector& { return alternatives; } private: std::string name; - std::vector>> alternatives; + std::vector alternatives; }; // Global variable definition implements the Declaration concept. diff --git a/executable_semantics/interpreter/interpreter.cpp b/executable_semantics/interpreter/interpreter.cpp index dd1daef4bb01..7ffca6716c4d 100644 --- a/executable_semantics/interpreter/interpreter.cpp +++ b/executable_semantics/interpreter/interpreter.cpp @@ -151,9 +151,9 @@ void Interpreter::InitEnv(const Declaration& d, Env* env) { case Declaration::Kind::ChoiceDeclaration: { const auto& choice = cast(d); VarValues alts; - for (const auto& [name, signature] : choice.Alternatives()) { - auto t = InterpExp(Env(arena), signature); - alts.push_back(make_pair(name, t)); + for (const auto& alternative : choice.Alternatives()) { + auto t = InterpExp(Env(arena), &alternative.signature()); + alts.push_back(make_pair(alternative.name(), t)); } auto ct = arena->New(choice.Name(), std::move(alts)); auto a = heap.AllocateValue(ct); diff --git a/executable_semantics/interpreter/type_checker.cpp b/executable_semantics/interpreter/type_checker.cpp index 8ecc68d5437f..82df4448bb05 100644 --- a/executable_semantics/interpreter/type_checker.cpp +++ b/executable_semantics/interpreter/type_checker.cpp @@ -1043,9 +1043,9 @@ void TypeChecker::TopLevel(const Declaration& d, TypeCheckContext* tops) { case Declaration::Kind::ChoiceDeclaration: { const auto& choice = cast(d); VarValues alts; - for (const auto& [name, signature] : choice.Alternatives()) { - auto t = interpreter.InterpExp(tops->values, signature); - alts.push_back(std::make_pair(name, t)); + for (const auto& alternative : choice.Alternatives()) { + auto t = interpreter.InterpExp(tops->values, &alternative.signature()); + alts.push_back(std::make_pair(alternative.name(), t)); } auto ct = arena->New(choice.Name(), std::move(alts)); Address a = interpreter.AllocateValue(ct); diff --git a/executable_semantics/syntax/BUILD b/executable_semantics/syntax/BUILD index ccfb4db827ed..adb7a63af58d 100644 --- a/executable_semantics/syntax/BUILD +++ b/executable_semantics/syntax/BUILD @@ -6,6 +6,12 @@ load("@mypy_integration//:mypy.bzl", "mypy_test") package(default_visibility = ["//executable_semantics:__pkg__"]) +cc_library( + name = "bison_wrap", + hdrs = ["bison_wrap.h"], + deps = ["//common:check"], +) + cc_library( name = "syntax", srcs = [ @@ -27,6 +33,7 @@ cc_library( "-Wno-writable-strings", ], deps = [ + ":bison_wrap", "//common:check", "//common:ostream", "//common:string_helpers", diff --git a/executable_semantics/syntax/bison_wrap.h b/executable_semantics/syntax/bison_wrap.h new file mode 100644 index 000000000000..bc835aa4ed79 --- /dev/null +++ b/executable_semantics/syntax/bison_wrap.h @@ -0,0 +1,44 @@ +// 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 + +#ifndef EXECUTABLE_SEMANTICS_SYNTAX_BISON_WRAP_H_ +#define EXECUTABLE_SEMANTICS_SYNTAX_BISON_WRAP_H_ + +#include + +#include "common/check.h" + +namespace Carbon { + +// Bison requires that types be default initializable for use with its variant +// semantics. This wraps arbitrary types to support a default constructor, while +// still requiring they be properly initialized. +template +class BisonWrap { + public: + // Assigning a value initializes the wrapper. + auto operator=(T&& rhs) -> BisonWrap& { + val = std::move(rhs); + return *this; + } + + // Support transparent conversion to the wrapped type. + operator T() { return Release(); } + + // Deliberately releases the contained value. Errors if not initialized. + // Called directly in parser.ypp when releasing pairs. + auto Release() -> T { + CHECK(val.has_value()); + T ret = std::move(*val); + val.reset(); + return ret; + } + + private: + std::optional val; +}; + +} // namespace Carbon + +#endif // EXECUTABLE_SEMANTICS_SYNTAX_BISON_WRAP_H_ diff --git a/executable_semantics/syntax/parser.ypp b/executable_semantics/syntax/parser.ypp index a95cbfd3d58a..5f19bb6f64a2 100644 --- a/executable_semantics/syntax/parser.ypp +++ b/executable_semantics/syntax/parser.ypp @@ -73,6 +73,7 @@ #include "executable_semantics/ast/pattern.h" #include "executable_semantics/common/arena.h" #include "executable_semantics/common/nonnull.h" + #include "executable_semantics/syntax/bison_wrap.h" namespace Carbon { class ParseAndLexContext; @@ -129,9 +130,9 @@ %type > paren_pattern_base %type ::Element> paren_pattern_element %type > paren_pattern_contents -%type >> alternative -%type >>> alternative_list -%type >>> alternative_list_contents +%type > alternative +%type > alternative_list +%type > alternative_list_contents %type , Nonnull>> clause %type , Nonnull>>> clause_list @@ -658,10 +659,10 @@ member_list: ; alternative: identifier tuple - { $$ = std::pair>($1, $2); } + { $$ = ChoiceDeclaration::Alternative($1, $2); } | identifier { - $$ = std::pair>( + $$ = ChoiceDeclaration::Alternative( $1, arena->New(context.SourceLoc())); } ; @@ -675,11 +676,11 @@ alternative_list: ; alternative_list_contents: alternative - { $$ = {$1}; } + { $$ = {std::move($1)}; } | alternative_list_contents COMMA alternative { $$ = $1; - $$.push_back($3); + $$.push_back(std::move($3)); } ; declaration: