From 0cb2c92c6c6f90a69c345b2a5830d9d694e8781c Mon Sep 17 00:00:00 2001 From: Adrien Leravat Date: Tue, 6 Dec 2022 13:31:18 -0800 Subject: [PATCH] Explorer: support index-based fields for `FieldPath` and `Members` (#2417) Features * Support addressing positional members (i.e. tuple fields) using an index. * Addresses a couple of `TODO`s relative to positional members Changes: * Add a new `IndexedValue` mirroring `NamedValue` * Adds `FieldPath::index() -> int` * Adds a `Member` variant for `IndexedValue` --- explorer/ast/member.cpp | 42 +++++++++++++++++++- explorer/ast/member.h | 29 +++++++++++++- explorer/interpreter/field_path.h | 8 +++- explorer/interpreter/interpreter.cpp | 12 ++---- explorer/interpreter/value.cpp | 57 +++++++++++++++++++++------- explorer/interpreter/value.h | 4 ++ 6 files changed, 124 insertions(+), 28 deletions(-) diff --git a/explorer/ast/member.cpp b/explorer/ast/member.cpp index 965671ca5dd4..cb5e82a1aaba 100644 --- a/explorer/ast/member.cpp +++ b/explorer/ast/member.cpp @@ -4,6 +4,7 @@ #include "explorer/ast/member.h" +#include "common/check.h" #include "explorer/ast/declaration.h" namespace Carbon { @@ -14,19 +15,46 @@ Member::Member(Nonnull declaration) Member::Member(Nonnull struct_member) : member_(struct_member) {} +Member::Member(Nonnull tuple_member) + : member_(tuple_member) {} + +auto Member::IsNamed(std::string_view other_name) const -> bool { + return HasName() && name() == other_name; +} + auto Member::name() const -> std::string_view { + CARBON_CHECK(HasName()) << "Unnamed member does not have a name()"; if (const auto* decl = member_.dyn_cast()) { return GetName(*decl).value(); + } else if (const auto* named_valued = member_.dyn_cast()) { + return named_valued->name; } else { - return member_.get()->name; + CARBON_FATAL() << "Unreachable"; } } +auto Member::HasPosition() const -> bool { + return member_.dyn_cast() != nullptr; +} + +auto Member::HasName() const -> bool { + // Both are currently mutually exclusive + return !HasPosition(); +} + +auto Member::index() const -> int { + CARBON_CHECK(HasPosition()) + << "Non-positional member does not have an index()"; + return member_.dyn_cast()->index; +} + auto Member::type() const -> const Value& { if (const auto* decl = member_.dyn_cast()) { return decl->static_type(); + } else if (const auto* named_valued = member_.dyn_cast()) { + return *named_valued->value; } else { - return *member_.get()->value; + return *member_.get()->value; } } @@ -37,4 +65,14 @@ auto Member::declaration() const -> std::optional> { return std::nullopt; } +void Member::Print(llvm::raw_ostream& out) const { + if (HasName()) { + out << name(); + } else if (const auto* value = member_.dyn_cast()) { + out << "element #" << member_.get()->index; + } else { + CARBON_FATAL() << "Unhandled member type"; + } +} + } // namespace Carbon diff --git a/explorer/ast/member.h b/explorer/ast/member.h index 470339fc9675..85272a4fa7d4 100644 --- a/explorer/ast/member.h +++ b/explorer/ast/member.h @@ -7,6 +7,7 @@ #include #include +#include #include "explorer/common/nonnull.h" #include "llvm/ADT/PointerUnion.h" @@ -25,6 +26,16 @@ struct NamedValue { Nonnull value; }; +// A IndexedValue represents a value identified by an index, such as a tuple +// field. +struct IndexedValue { + // The field index. + int index; + + // The field's value. + Nonnull value; +}; + // A member of a type. // // This is either a declared member of a class, interface, or similar, or a @@ -33,16 +44,30 @@ class Member { public: explicit Member(Nonnull declaration); explicit Member(Nonnull struct_member); + explicit Member(Nonnull tuple_member); - // The name of the member. + // Return whether the member's name matches `name`. + auto IsNamed(std::string_view name) const -> bool; + // Prints the Member + void Print(llvm::raw_ostream& out) const; + + // Return whether the member is positional, i.e. has an index. + auto HasPosition() const -> bool; + // Return whether the member is named, i.e. has a name. + auto HasName() const -> bool; + + // The name of the member. Requires *this to represent a named member. auto name() const -> std::string_view; + // The index of the member. Requires *this to represent a positional member. + auto index() const -> int; // The declared type of the member, which might include type variables. auto type() const -> const Value&; // A declaration of the member, if any exists. auto declaration() const -> std::optional>; private: - llvm::PointerUnion, Nonnull> + llvm::PointerUnion, Nonnull, + Nonnull> member_; }; diff --git a/explorer/interpreter/field_path.h b/explorer/interpreter/field_path.h index 07e973c6a18a..ccc484d89130 100644 --- a/explorer/interpreter/field_path.h +++ b/explorer/interpreter/field_path.h @@ -5,7 +5,9 @@ #ifndef CARBON_EXPLORER_INTERPRETER_FIELD_PATH_H_ #define CARBON_EXPLORER_INTERPRETER_FIELD_PATH_H_ +#include #include +#include #include #include "common/ostream.h" @@ -48,7 +50,9 @@ class FieldPath { auto member() const -> Member { return member_; } - auto name() const -> std::string_view { return member_.name(); } + auto IsNamed(std::string_view name) const -> bool { + return member_.IsNamed(name); + } auto interface() const -> std::optional> { return interface_; @@ -58,7 +62,7 @@ class FieldPath { return witness_; } - void Print(llvm::raw_ostream& out) const { out << name(); } + void Print(llvm::raw_ostream& out) const { return member_.Print(out); } private: Member member_; diff --git a/explorer/interpreter/interpreter.cpp b/explorer/interpreter/interpreter.cpp index 5a556c3712f9..2aa69a975b60 100644 --- a/explorer/interpreter/interpreter.cpp +++ b/explorer/interpreter/interpreter.cpp @@ -463,14 +463,10 @@ auto Interpreter::StepLvalue() -> ErrorOr { // { v :: [][i] :: C, E, F} :: S, H} // -> { { &v[i] :: C, E, F} :: S, H } Address object = cast(*act.results()[0]).address(); - // TODO: Add support to `Member` for naming tuple fields rather than - // pretending we have struct fields with numerical names. - std::string f = - std::to_string(cast(*act.results()[1]).value()); - auto* tuple_field_as_struct_field = - arena_->New(NamedValue{f, &exp.static_type()}); - Address field = - object.SubobjectAddress(Member(tuple_field_as_struct_field)); + const auto index = cast(*act.results()[1]).value(); + auto* tuple_field = + arena_->New(IndexedValue{index, &exp.static_type()}); + Address field = object.SubobjectAddress(Member(tuple_field)); return todo_.FinishAction(arena_->New(field)); } } diff --git a/explorer/interpreter/value.cpp b/explorer/interpreter/value.cpp index faeb39b0dc72..bf2898efd09a 100644 --- a/explorer/interpreter/value.cpp +++ b/explorer/interpreter/value.cpp @@ -14,6 +14,7 @@ #include "explorer/common/arena.h" #include "explorer/common/error_builders.h" #include "explorer/interpreter/action.h" +#include "explorer/interpreter/field_path.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Casting.h" @@ -48,12 +49,32 @@ static auto FindClassField(Nonnull object, return std::nullopt; } -static auto GetMember(Nonnull arena, Nonnull v, - const FieldPath::Component& field, - SourceLocation source_loc, Nonnull me_value) +static auto GetPositionalMember(Nonnull v, + const FieldPath::Component& field, + SourceLocation source_loc) -> ErrorOr> { - std::string_view f = field.name(); + switch (v->kind()) { + case Value::Kind::TupleValue: { + const auto& tuple = cast(*v); + const auto index = field.member().index(); + if (index < 0 || index >= static_cast(tuple.elements().size())) { + return ProgramError(source_loc) + << "index " << index << " out of range for " << *v; + } + return tuple.elements()[index]; + } + default: + return ProgramError(source_loc) + << "Invalid positional argument for value " << *v; + } +} +static auto GetNamedMember(Nonnull arena, Nonnull v, + const FieldPath::Component& field, + SourceLocation source_loc, + Nonnull me_value) + -> ErrorOr> { + const auto f = field.member().name(); if (field.witness().has_value()) { const auto* witness = cast(*field.witness()); @@ -152,6 +173,15 @@ static auto GetMember(Nonnull arena, Nonnull v, } } +static auto GetMember(Nonnull arena, Nonnull v, + const FieldPath::Component& field, + SourceLocation source_loc, Nonnull me_value) + -> ErrorOr> { + return field.member().HasPosition() + ? GetPositionalMember(v, field, source_loc) + : GetNamedMember(arena, v, field, source_loc, me_value); +} + auto Value::GetMember(Nonnull arena, const FieldPath& path, SourceLocation source_loc, Nonnull me_value) const @@ -178,11 +208,11 @@ static auto SetFieldImpl( std::vector elements = cast(*value).elements(); auto it = llvm::find_if(elements, [path_begin](const NamedValue& element) { - return element.name == (*path_begin).name(); + return (*path_begin).IsNamed(element.name); }); if (it == elements.end()) { return ProgramError(source_loc) - << "field " << (*path_begin).name() << " not in " << *value; + << "field " << *path_begin << " not in " << *value; } CARBON_ASSIGN_OR_RETURN( it->value, SetFieldImpl(arena, it->value, path_begin + 1, path_end, @@ -207,17 +237,16 @@ static auto SetFieldImpl( } // Failed to match, show full object content return ProgramError(source_loc) - << "field " << (*path_begin).name() << " not in " << *value; + << "field " << *path_begin << " not in " << *value; } case Value::Kind::TupleType: case Value::Kind::TupleValue: { std::vector> elements = cast(*value).elements(); - // TODO(geoffromer): update FieldPath to hold integers as well as strings. - int index = std::stoi(std::string((*path_begin).name())); - if (index < 0 || static_cast(index) >= elements.size()) { - return ProgramError(source_loc) << "index " << (*path_begin).name() - << " out of range in " << *value; + const auto index = (*path_begin).member().index(); + if (index < 0 || index >= static_cast(elements.size())) { + return ProgramError(source_loc) + << "index " << index << " out of range in " << *value; } CARBON_ASSIGN_OR_RETURN( elements[index], SetFieldImpl(arena, elements[index], path_begin + 1, @@ -541,7 +570,7 @@ void Value::Print(llvm::raw_ostream& out) const { if (member_name.interface().has_value()) { out << *member_name.interface().value(); } - out << "." << member_name.name(); + out << "." << member_name; if (member_name.base_type().has_value() && member_name.interface().has_value()) { out << ")"; @@ -587,7 +616,7 @@ void Value::Print(llvm::raw_ostream& out) const { << cast(*this).name(); break; case Value::Kind::TypeOfMemberName: { - out << "member name " << cast(*this).member().name(); + out << "member name " << cast(*this).member(); break; } case Value::Kind::StaticArrayType: { diff --git a/explorer/interpreter/value.h b/explorer/interpreter/value.h index 089e165f4ac2..d5ff558fc7de 100644 --- a/explorer/interpreter/value.h +++ b/explorer/interpreter/value.h @@ -1146,12 +1146,16 @@ class MemberName : public Value { member_(member) { CARBON_CHECK(base_type || interface) << "member name must be in a type, an interface, or both"; + CARBON_CHECK(member_.HasName()) << "member must have a name"; } static auto classof(const Value* value) -> bool { return value->kind() == Kind::MemberName; } + // Prints the member name or identifier. + void Print(llvm::raw_ostream& out) const { member_.Print(out); } + // The type for which `name` is a member or a member of an `impl`. auto base_type() const -> std::optional> { return base_type_;