From ca05ef9d123503aecfc2910c8abf492e6384dd7b Mon Sep 17 00:00:00 2001 From: dank-openai Date: Thu, 16 Jul 2026 17:04:27 -0400 Subject: [PATCH 1/4] dslx: replace NameDefTree with PatternTree --- xls/dslx/bytecode/bytecode.h | 2 +- xls/dslx/bytecode/bytecode_emitter.cc | 89 +++--- xls/dslx/bytecode/bytecode_emitter.h | 6 +- xls/dslx/bytecode/bytecode_emitter_test.cc | 8 +- .../bytecode/bytecode_interpreter_test.cc | 16 +- .../exhaustiveness_match_test.cc | 16 +- .../match_exhaustiveness_checker.cc | 107 ++++--- .../match_exhaustiveness_checker.h | 2 +- xls/dslx/fmt/ast_fmt.cc | 57 ++-- xls/dslx/fmt/ast_fmt.h | 7 +- xls/dslx/frontend/ast.cc | 280 ++++++++++++++---- xls/dslx/frontend/ast.h | 209 ++++++------- xls/dslx/frontend/ast_cloner.cc | 61 ++-- xls/dslx/frontend/ast_cloner_test.cc | 58 +++- xls/dslx/frontend/ast_node.h | 2 +- xls/dslx/frontend/ast_test.cc | 28 +- xls/dslx/frontend/ast_utils_test.cc | 51 ++-- xls/dslx/frontend/function_specializer.cc | 4 +- xls/dslx/frontend/parser.cc | 127 ++++---- xls/dslx/frontend/parser.h | 24 +- xls/dslx/frontend/parser_test.cc | 106 ++++++- xls/dslx/frontend/semantics_analysis.cc | 27 +- xls/dslx/ir_convert/function_converter.cc | 178 ++++++----- xls/dslx/ir_convert/function_converter.h | 6 +- .../ir_convert/proc_config_ir_converter.cc | 6 +- xls/dslx/lsp/language_server_adapter.cc | 12 +- xls/dslx/lsp/language_server_adapter_test.cc | 28 +- xls/dslx/type_system/deduce_utils.cc | 77 ++--- xls/dslx/type_system/deduce_utils.h | 27 +- xls/dslx/type_system/type_info.proto | 2 + xls/dslx/type_system/type_info_to_proto.cc | 10 +- .../type_system/type_info_to_proto_test.cc | 35 ++- xls/dslx/type_system/typecheck_module_test.cc | 8 +- xls/dslx/type_system_v2/constant_collector.cc | 50 ++-- xls/dslx/type_system_v2/decorate_error.cc | 19 +- .../type_system_v2/flatten_in_type_order.cc | 10 +- .../inference_table_converter_impl.cc | 2 +- .../type_system_v2/populate_table_visitor.cc | 133 ++++----- .../typecheck_module_v2_array_tuple_test.cc | 8 +- .../typecheck_module_v2_error_handler_test.cc | 6 +- .../type_system_v2/validate_concrete_type.cc | 6 +- xls/fuzzer/ast_generator.cc | 78 +++-- xls/fuzzer/ast_generator.h | 7 +- 43 files changed, 1155 insertions(+), 840 deletions(-) diff --git a/xls/dslx/bytecode/bytecode.h b/xls/dslx/bytecode/bytecode.h index 65e299ac8a..f67daa38cc 100644 --- a/xls/dslx/bytecode/bytecode.h +++ b/xls/dslx/bytecode/bytecode.h @@ -230,7 +230,7 @@ class Bytecode { std::optional callee_bindings_; }; - // Encapsulates an element in a MatchArm's NameDefTree. For literals, a + // Encapsulates an element in a MatchArm's pattern. For literals, a // this is an InterpValue. For NameRefs, this is the associated SlotIndex. For // NameDefs (i.e., assignments to a name from the value to match), this is the // SlotIndex to which to store, and for wildcards, this is a simple "matches diff --git a/xls/dslx/bytecode/bytecode_emitter.cc b/xls/dslx/bytecode/bytecode_emitter.cc index f3b8498230..ac7758e567 100644 --- a/xls/dslx/bytecode/bytecode_emitter.cc +++ b/xls/dslx/bytecode/bytecode_emitter.cc @@ -954,7 +954,7 @@ absl::Status BytecodeEmitter::HandleFor(const For* node) { bytecode_.push_back(Bytecode(node->span(), Bytecode::Op::kSwap)); bytecode_.push_back(Bytecode(node->span(), Bytecode::Op::kCreateTuple, Bytecode::NumElements(2))); - XLS_RETURN_IF_ERROR(DestructureLet(node->names(), /*type_or_size=*/2)); + XLS_RETURN_IF_ERROR(DestructureLet(node->pattern(), /*type_or_size=*/2)); // Emit the loop body. XLS_RETURN_IF_ERROR(node->body()->AcceptExpr(this)); @@ -1252,9 +1252,9 @@ absl::Status BytecodeEmitter::HandleInvocation(const Invocation* node) { return absl::OkStatus(); } -absl::StatusOr BytecodeEmitter::HandleNameDefTreeExpr( - NameDefTree* tree, Type* type) { - if (tree->is_leaf()) { +absl::StatusOr BytecodeEmitter::HandlePatternExpr( + const PatternTree& pattern, Type* type) { + if (!std::holds_alternative(pattern)) { return absl::visit( Visitor{ [&](NameRef* n) -> absl::StatusOr { @@ -1303,32 +1303,38 @@ absl::StatusOr BytecodeEmitter::HandleNameDefTreeExpr( [&](RestOfTuple* n) -> absl::StatusOr { return Bytecode::MatchArmItem::MakeRestOfTuple(); }, + [&](TuplePattern* n) -> absl::StatusOr { + return absl::InternalError("Tuple pattern reached leaf handler"); + }, }, - tree->leaf()); + pattern); } - // Not a leaf; must be a tuple + TuplePattern* tuple_pattern = std::get(pattern); auto* tuple_type = absl::down_cast(type); if (tuple_type == nullptr) { return TypeInferenceErrorStatus( - tree->span(), type, "Pattern expected matched-on type to be a tuple.", - file_table()); + tuple_pattern->span(), type, + "Pattern expected matched-on type to be a tuple.", file_table()); } - XLS_ASSIGN_OR_RETURN((auto [number_of_tuple_elements, number_of_names]), - GetTupleSizes(tree, tuple_type)); + XLS_ASSIGN_OR_RETURN( + (auto [number_of_tuple_elements, non_rest_pattern_member_count]), + GetTupleSizes(tuple_pattern, tuple_type)); // TODO: https://github.com/google/xls/issues/1459 - This is at least the // 3rd if not 4th time a loop like this has been written. It should be // refactored into a common utility function. std::vector elements; int64_t tuple_index = 0; - const NameDefTree::Nodes& nodes = tree->nodes(); - for (int64_t name_index = 0; name_index < nodes.size(); ++name_index) { - NameDefTree* subnode = nodes[name_index]; - if (subnode->IsRestOfTupleLeaf()) { + const std::vector& members = tuple_pattern->members(); + for (int64_t member_index = 0; member_index < members.size(); + ++member_index) { + const PatternTree& subpattern = members[member_index]; + if (IsRestOfTupleLeaf(subpattern)) { // Skip ahead. - int64_t wildcards_to_insert = number_of_tuple_elements - number_of_names; + int64_t wildcards_to_insert = + number_of_tuple_elements - non_rest_pattern_member_count; tuple_index += wildcards_to_insert; for (int64_t i = 0; i < wildcards_to_insert; ++i) { @@ -1339,7 +1345,7 @@ absl::StatusOr BytecodeEmitter::HandleNameDefTreeExpr( Type& subtype = tuple_type->GetMemberType(tuple_index); XLS_ASSIGN_OR_RETURN(Bytecode::MatchArmItem element, - HandleNameDefTreeExpr(subnode, &subtype)); + HandlePatternExpr(subpattern, &subtype)); elements.push_back(element); tuple_index++; } @@ -1360,25 +1366,26 @@ static int64_t CountElements(std::variant element) { } absl::Status BytecodeEmitter::DestructureLet( - NameDefTree* tree, std::variant type_or_size) { - if (tree->is_leaf()) { - if (std::holds_alternative(tree->leaf()) || - std::holds_alternative(tree->leaf())) { + const PatternTree& pattern, std::variant type_or_size) { + if (!std::holds_alternative(pattern)) { + if (IsWildcardLeaf(pattern) || IsRestOfTupleLeaf(pattern)) { // We can just drop this one. - Add(Bytecode::MakePop(tree->span())); + Add(Bytecode::MakePop(GetPatternSpan(pattern))); return absl::OkStatus(); } - NameDef* name_def = std::get(tree->leaf()); + NameDef* name_def = std::get(pattern); if (!namedef_to_slot_.contains(name_def)) { namedef_to_slot_.insert({name_def, next_slotno_++}); } int64_t slot = namedef_to_slot_.at(name_def); - Add(Bytecode::MakeStore(tree->span(), Bytecode::SlotIndex(slot))); + Add(Bytecode::MakeStore(GetPatternSpan(pattern), + Bytecode::SlotIndex(slot))); } else { + TuplePattern* tuple_pattern = std::get(pattern); // Pushes each element of the current level of the tuple // onto the stack in reverse order, e.g., (a, (b, c)) pushes (b, c) then a - Add(Bytecode(tree->span(), Bytecode::Op::kExpandTuple)); + Add(Bytecode(tuple_pattern->span(), Bytecode::Op::kExpandTuple)); // Note: we intentionally don't check validity of the tuple here; that's // done by Deduce(). @@ -1393,21 +1400,23 @@ absl::Status BytecodeEmitter::DestructureLet( } int64_t tuple_index = 0; - for (int64_t name_index = 0; name_index < tree->nodes().size(); - ++name_index) { - NameDefTree* node = tree->nodes()[name_index]; - if (node->IsRestOfTupleLeaf()) { + for (int64_t member_index = 0; + member_index < tuple_pattern->members().size(); ++member_index) { + const PatternTree& member = tuple_pattern->members()[member_index]; + if (IsRestOfTupleLeaf(member)) { int64_t number_of_tuple_elements = CountElements(type_or_size); // Decrement for the rest-of-tuple - int64_t number_of_bindings = tree->nodes().size() - 1; + int64_t non_rest_pattern_member_count = + tuple_pattern->members().size() - 1; // Skip ahead to account for the needed remaining elements. - int64_t difference = number_of_tuple_elements - number_of_bindings; + int64_t difference = + number_of_tuple_elements - non_rest_pattern_member_count; tuple_index += difference; // Pop unused tuple elements for (int64_t pop_count = 0; pop_count < difference; ++pop_count) { - Add(Bytecode::MakePop(node->span())); + Add(Bytecode::MakePop(GetPatternSpan(member))); } continue; } @@ -1415,13 +1424,13 @@ absl::Status BytecodeEmitter::DestructureLet( Visitor{[&](Type* type) -> absl::Status { TupleType* tuple_type = absl::down_cast(type); return DestructureLet( - node, &tuple_type->GetMemberType(tuple_index)); + member, &tuple_type->GetMemberType(tuple_index)); }, [&](int64_t size) -> absl::Status { // If a simple count is given, the tuple can only // contain single elements, so the child count // must be 1. - return DestructureLet(node, 1); + return DestructureLet(member, 1); }}, type_or_size)); ++tuple_index; @@ -1438,7 +1447,7 @@ absl::Status BytecodeEmitter::HandleLet(const Let* node) { XLS_RETURN_IF_ERROR(node->rhs()->AcceptExpr(this)); std::optional type = type_info_->GetItem(node->rhs()); if (type.has_value()) { - return DestructureLet(node->name_def_tree(), type.value()); + return DestructureLet(node->pattern(), type.value()); } return absl::InternalError(absl::StrFormat( "@ %s: Could not retrieve type of right-hand side of `let`.", @@ -1877,7 +1886,7 @@ absl::Status BytecodeEmitter::HandleMatch(const Match* node) { Add(Bytecode::MakeJumpDest(node->span())); } - const std::vector& patterns = arm->patterns(); + const std::vector& patterns = arm->patterns(); // First, prime the stack with all the copies of the matchee we'll need. for (int pattern_idx = 0; pattern_idx < patterns.size(); pattern_idx++) { Add(Bytecode::MakeDup(node->matched()->span())); @@ -1887,16 +1896,16 @@ absl::Status BytecodeEmitter::HandleMatch(const Match* node) { // Then we match each arm. We OR with the prev. result (if there is one) // and swap to the next copy of the matchee, unless this is the last // pattern. - NameDefTree* ndt = arm->patterns()[pattern_idx]; + const PatternTree& pattern = arm->patterns()[pattern_idx]; XLS_ASSIGN_OR_RETURN(Bytecode::MatchArmItem arm_item, - HandleNameDefTreeExpr(ndt, type.value())); - Add(Bytecode::MakeMatchArm(ndt->span(), arm_item)); + HandlePatternExpr(pattern, type.value())); + Add(Bytecode::MakeMatchArm(GetPatternSpan(pattern), arm_item)); if (pattern_idx != 0) { - Add(Bytecode::MakeLogicalOr(ndt->span())); + Add(Bytecode::MakeLogicalOr(GetPatternSpan(pattern))); } if (pattern_idx != patterns.size() - 1) { - Add(Bytecode::MakeSwap(ndt->span())); + Add(Bytecode::MakeSwap(GetPatternSpan(pattern))); } } Add(Bytecode::MakeInvert(arm->span())); diff --git a/xls/dslx/bytecode/bytecode_emitter.h b/xls/dslx/bytecode/bytecode_emitter.h index fe2b819cf4..ca7b771459 100644 --- a/xls/dslx/bytecode/bytecode_emitter.h +++ b/xls/dslx/bytecode/bytecode_emitter.h @@ -189,10 +189,10 @@ class BytecodeEmitter : public ExprVisitor { absl::StatusOr HandleColonRefToValue(Module* module, const ColonRef* colon_ref); - absl::StatusOr HandleNameDefTreeExpr( - NameDefTree* tree, Type* type = nullptr); + absl::StatusOr HandlePatternExpr( + const PatternTree& pattern, Type* type = nullptr); - absl::Status DestructureLet(NameDefTree* tree, + absl::Status DestructureLet(const PatternTree& pattern, std::variant type_or_size); const FileTable& file_table() const { return import_data_->file_table(); } diff --git a/xls/dslx/bytecode/bytecode_emitter_test.cc b/xls/dslx/bytecode/bytecode_emitter_test.cc index 7ece83c5f2..9d03bd23e4 100644 --- a/xls/dslx/bytecode/bytecode_emitter_test.cc +++ b/xls/dslx/bytecode/bytecode_emitter_test.cc @@ -21,8 +21,6 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/algorithm/container.h" #include "absl/base/casts.h" #include "absl/container/flat_hash_map.h" @@ -31,6 +29,8 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" #include "absl/strings/str_split.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" #include "re2/re2.h" #include "xls/common/status/matchers.h" #include "xls/common/status/status_macros.h" @@ -142,7 +142,7 @@ fn expect_fail() -> u32 { TEST(BytecodeEmitterTest, DestructuringLet) { constexpr std::string_view kProgram = R"( -fn has_name_def_tree() -> (u32, u64, uN[128]) { +fn has_tuple_pattern() -> (u32, u64, uN[128]) { let (a, b, (c, d)) = (u4:0, u8:1, (u16:2, (u32:3, u64:4, uN[128]:5))); assert_eq(a, u4:0); assert_eq(b, u8:1); @@ -154,7 +154,7 @@ fn has_name_def_tree() -> (u32, u64, uN[128]) { ImportData import_data(CreateImportDataForTest()); XLS_ASSERT_OK_AND_ASSIGN( std::unique_ptr bf, - EmitBytecodes(&import_data, kProgram, "has_name_def_tree")); + EmitBytecodes(&import_data, kProgram, "has_tuple_pattern")); EXPECT_EQ(BytecodesToString(bf->bytecodes(), /*source_locs=*/false, import_data.file_table()), diff --git a/xls/dslx/bytecode/bytecode_interpreter_test.cc b/xls/dslx/bytecode/bytecode_interpreter_test.cc index e44f04d619..3b65ca595f 100644 --- a/xls/dslx/bytecode/bytecode_interpreter_test.cc +++ b/xls/dslx/bytecode/bytecode_interpreter_test.cc @@ -22,12 +22,12 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/status_matchers.h" #include "absl/status/statusor.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" #include "xls/common/status/matchers.h" #include "xls/common/status/ret_check.h" #include "xls/common/status/status_macros.h" @@ -739,7 +739,7 @@ fn main() { } TEST_F(BytecodeInterpreterTest, DestructuringLet) { constexpr std::string_view kProgram = R"( -fn has_name_def_tree() -> (u32, u64, uN[128]) { +fn has_tuple_pattern() -> (u32, u64, uN[128]) { let (a, b, (c, d)) = (u4:0, u8:1, (u16:2, (u32:3, u64:4, uN[128]:5))); assert_eq(a, u4:0); assert_eq(b, u8:1); @@ -749,7 +749,7 @@ fn has_name_def_tree() -> (u32, u64, uN[128]) { })"; XLS_ASSERT_OK_AND_ASSIGN(InterpValue value, - Interpret(kProgram, "has_name_def_tree")); + Interpret(kProgram, "has_tuple_pattern")); ASSERT_TRUE(value.IsTuple()); XLS_ASSERT_OK_AND_ASSIGN(int64_t num_elements, value.GetLength()); @@ -770,7 +770,7 @@ fn has_name_def_tree() -> (u32, u64, uN[128]) { TEST_F(BytecodeInterpreterTest, DestructuringLetWithRestOfTuple) { constexpr std::string_view kProgram = R"( -fn has_name_def_tree() -> (u32, u64, uN[128]) { +fn has_tuple_pattern() -> (u32, u64, uN[128]) { let (a, b, .., (c, .., d)) = (u4:0, u8:1, u9:2, u10:3, (u16:2, u17:2, (u32:3, u64:4, uN[128]:5))); assert_eq(a, u4:0); assert_eq(b, u8:1); @@ -780,7 +780,7 @@ fn has_name_def_tree() -> (u32, u64, uN[128]) { })"; XLS_ASSERT_OK_AND_ASSIGN(InterpValue value, - Interpret(kProgram, "has_name_def_tree")); + Interpret(kProgram, "has_tuple_pattern")); ASSERT_TRUE(value.IsTuple()); XLS_ASSERT_OK_AND_ASSIGN(int64_t num_elements, value.GetLength()); @@ -801,7 +801,7 @@ fn has_name_def_tree() -> (u32, u64, uN[128]) { TEST_F(BytecodeInterpreterTest, DestructuringLetWithRestOfTupleSkipsZero) { constexpr std::string_view kProgram = R"( -fn has_name_def_tree() -> (u32, u64, uN[128]) { +fn has_tuple_pattern() -> (u32, u64, uN[128]) { let (a, b, .., (c, .., d)) = (u4:0, u8:1, (u16:2, (u32:3, u64:4, uN[128]:5))); assert_eq(a, u4:0); assert_eq(b, u8:1); @@ -811,7 +811,7 @@ fn has_name_def_tree() -> (u32, u64, uN[128]) { })"; XLS_ASSERT_OK_AND_ASSIGN(InterpValue value, - Interpret(kProgram, "has_name_def_tree")); + Interpret(kProgram, "has_tuple_pattern")); ASSERT_TRUE(value.IsTuple()); XLS_ASSERT_OK_AND_ASSIGN(int64_t num_elements, value.GetLength()); diff --git a/xls/dslx/exhaustiveness/exhaustiveness_match_test.cc b/xls/dslx/exhaustiveness/exhaustiveness_match_test.cc index fdf3163ef3..736a12c061 100644 --- a/xls/dslx/exhaustiveness/exhaustiveness_match_test.cc +++ b/xls/dslx/exhaustiveness/exhaustiveness_match_test.cc @@ -21,13 +21,13 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/container/flat_hash_map.h" #include "absl/status/status.h" #include "absl/status/status_matchers.h" #include "absl/status/statusor.h" #include "absl/types/span.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" #include "xls/common/status/matchers.h" #include "xls/dslx/create_import_data.h" #include "xls/dslx/exhaustiveness/match_exhaustiveness_checker.h" @@ -46,10 +46,10 @@ namespace { using ::absl_testing::StatusIs; using ::testing::HasSubstr; -std::vector GetPatterns(const Match& match) { - std::vector patterns; +std::vector GetPatterns(const Match& match) { + std::vector patterns; for (const MatchArm* arm : match.arms()) { - for (const NameDefTree* pattern : arm->patterns()) { + for (const PatternTree& pattern : arm->patterns()) { patterns.push_back(pattern); } } @@ -75,15 +75,15 @@ void CheckExhaustiveOnlyAfterLastPattern(std::string_view program) { MatchExhaustivenessChecker checker(match->matched()->span(), import_data, *tm.type_info, *matched_type.value()); - std::vector patterns = GetPatterns(*match); + std::vector patterns = GetPatterns(*match); for (int64_t i = 0; i < patterns.size(); ++i) { - bool now_exhaustive = checker.AddPattern(*patterns[i]); + bool now_exhaustive = checker.AddPattern(patterns[i]); // We expect it to become exhaustive with the last match arm. bool expect_now_exhaustive = i + 1 == patterns.size(); EXPECT_EQ(now_exhaustive, expect_now_exhaustive) << "Expected match to be " << (expect_now_exhaustive ? "exhaustive" : "non-exhaustive") - << " after adding pattern `" << patterns[i]->ToString() << "`"; + << " after adding pattern `" << PatternToString(patterns[i]) << "`"; } } diff --git a/xls/dslx/exhaustiveness/match_exhaustiveness_checker.cc b/xls/dslx/exhaustiveness/match_exhaustiveness_checker.cc index ca78ad677c..5c69478ad1 100644 --- a/xls/dslx/exhaustiveness/match_exhaustiveness_checker.cc +++ b/xls/dslx/exhaustiveness/match_exhaustiveness_checker.cc @@ -69,8 +69,8 @@ std::vector GetLeafTypes(const Type& type, const Span& span, // WildcardPattern and NameDef. struct SomeWildcard {}; -// NameDefTree::Leaf but where RestOfTuple has been resolved. -using PatternLeaf = +// PatternLeaf but where RestOfTuple has been resolved. +using IntervalPatternLeaf = std::variant; InterpValueInterval MakeFullIntervalForType(const Type& type) { @@ -125,8 +125,8 @@ InterpValueInterval MakeIntervalForType(const Type& type, } std::optional PatternToIntervalInternal( - const PatternLeaf& leaf, const Type& leaf_type, const TypeInfo& type_info, - const ImportData& import_data) { + const IntervalPatternLeaf& leaf, const Type& leaf_type, + const TypeInfo& type_info, const ImportData& import_data) { std::optional result = absl::visit( Visitor{ [&](SomeWildcard /*unused*/) -> std::optional { @@ -191,40 +191,47 @@ std::optional PatternToIntervalInternal( return result; } -PatternLeaf ToPatternLeaf(const NameDefTree::Leaf& leaf) { +IntervalPatternLeaf ToIntervalPatternLeaf(const PatternTree& pattern) { return absl::visit( Visitor{ - [&](NameDef* name_def) -> PatternLeaf { return SomeWildcard(); }, - [&](NameRef* name_ref) -> PatternLeaf { return name_ref; }, - [&](Range* range) -> PatternLeaf { return range; }, - [&](ColonRef* colon_ref) -> PatternLeaf { return colon_ref; }, - [&](WildcardPattern* wildcard_pattern) -> PatternLeaf { + [&](NameDef* name_def) -> IntervalPatternLeaf { + return SomeWildcard(); + }, + [&](NameRef* name_ref) -> IntervalPatternLeaf { return name_ref; }, + [&](Range* range) -> IntervalPatternLeaf { return range; }, + [&](ColonRef* colon_ref) -> IntervalPatternLeaf { return colon_ref; }, + [&](WildcardPattern* wildcard_pattern) -> IntervalPatternLeaf { + return SomeWildcard(); + }, + [&](Number* number) -> IntervalPatternLeaf { return number; }, + [&](RestOfTuple* rest_of_tuple) -> IntervalPatternLeaf { + LOG(FATAL) << "RestOfTuple not valid for conversion to " + "IntervalPatternLeaf"; return SomeWildcard(); }, - [&](Number* number) -> PatternLeaf { return number; }, - [&](RestOfTuple* rest_of_tuple) -> PatternLeaf { - LOG(FATAL) << "RestOfTuple not valid for conversion to PatternLeaf"; + [&](TuplePattern* /*unused*/) -> IntervalPatternLeaf { + LOG(FATAL) << "TuplePattern not valid for conversion to " + "IntervalPatternLeaf"; + return SomeWildcard(); }}, - leaf); + pattern); } -std::vector ExpandPatternLeaves(const NameDefTree& pattern, - const Type& type, - const FileTable& file_table) { - VLOG(5) << "ExpandPatternLeaves; pattern: `" << pattern.ToString() +std::vector ExpandPatternLeaves( + const PatternTree& pattern, const Type& type, const FileTable& file_table) { + VLOG(5) << "ExpandPatternLeaves; pattern: `" << PatternToString(pattern) << "` type: `" << type.ToString() << "`"; // For an irrefutable pattern, simply return wildcards for every leaf. - if (pattern.IsIrrefutable()) { + if (IsIrrefutablePattern(pattern)) { std::vector leaf_types = - GetLeafTypes(type, pattern.span(), file_table); - return std::vector(leaf_types.size(), SomeWildcard()); + GetLeafTypes(type, GetPatternSpan(pattern), file_table); + return std::vector(leaf_types.size(), SomeWildcard()); } // If the type is not a tuple then we expect the pattern to be a single leaf. if (!type.IsTuple()) { - std::vector leaves = pattern.Flatten(); - CHECK_EQ(leaves.size(), 1) - << "Expected a single leaf for non-tuple type, got " << leaves.size(); - return {ToPatternLeaf(leaves.front())}; + CHECK(!std::holds_alternative(pattern)) + << "Expected a single leaf for non-tuple type"; + return {ToIntervalPatternLeaf(pattern)}; } // Walk through the pattern and expand any RestOfTuple markers into the // appropriate number of wildcards. @@ -233,8 +240,7 @@ std::vector ExpandPatternLeaves(const NameDefTree& pattern, // any sub-tuples encountered. absl::Span> tuple_members = type.AsTuple().members(); - std::vector> flattened = - pattern.Flatten1(); + std::vector flattened = FlattenPattern1(pattern); // Note: there can be fewer flatten1'd nodes than tuple elements because of // RestOfTuple markers. @@ -244,7 +250,7 @@ std::vector ExpandPatternLeaves(const NameDefTree& pattern, CHECK_LE(flattened.size(), tuple_members.size() + 1); // The results correspond to leaf types. - std::vector result; + std::vector result; // The tuple type index at *this level* of the tuple. // We bump this as we progress through -- note a single "flattened_index" @@ -261,36 +267,34 @@ std::vector ExpandPatternLeaves(const NameDefTree& pattern, << "Flattened index out of bounds."; const auto& node = flattened[flattened_index]; - if (std::holds_alternative(node)) { - const NameDefTree* sub_pattern = std::get(node); + if (std::holds_alternative(node)) { CHECK_LT(types_index, tuple_members.size()); const Type& type_at_index = *tuple_members[types_index]; - std::vector sub_pattern_leaves = - ExpandPatternLeaves(*sub_pattern, type_at_index, file_table); + std::vector sub_pattern_leaves = + ExpandPatternLeaves(node, type_at_index, file_table); result.insert(result.end(), sub_pattern_leaves.begin(), sub_pattern_leaves.end()); types_index += 1; continue; } - const NameDefTree::Leaf& leaf = std::get(node); absl::visit( Visitor{ [&](const NameRef* n) { - result.push_back(ToPatternLeaf(leaf)); + result.push_back(ToIntervalPatternLeaf(node)); types_index += 1; }, [&](const Range* r) { - result.push_back(ToPatternLeaf(leaf)); + result.push_back(ToIntervalPatternLeaf(node)); types_index += 1; }, [&](const ColonRef* c) { - result.push_back(ToPatternLeaf(leaf)); + result.push_back(ToIntervalPatternLeaf(node)); types_index += 1; }, [&](const Number* n) { - result.push_back(ToPatternLeaf(leaf)); + result.push_back(ToIntervalPatternLeaf(node)); types_index += 1; }, [&](const RestOfTuple* /*unused*/) { @@ -310,7 +314,8 @@ std::vector ExpandPatternLeaves(const NameDefTree& pattern, CHECK_LT(types_index, tuple_members.size()); const Type& type_at_index = *tuple_members[types_index]; for (int64_t i = 0; - i < GetLeafTypes(type_at_index, pattern.span(), file_table) + i < GetLeafTypes(type_at_index, GetPatternSpan(pattern), + file_table) .size(); ++i) { result.push_back(SomeWildcard()); @@ -322,34 +327,39 @@ std::vector ExpandPatternLeaves(const NameDefTree& pattern, << flattened_index << " types_index: " << types_index << " result.size(): " << result.size(); }, + [&](const TuplePattern*) { + LOG(FATAL) << "TuplePattern reached leaf handler"; + }, [&](const auto* irrefutable_leaf) { // Push back wildcards of the right size for the type. CHECK_LT(types_index, tuple_members.size()); const Type& type_at_index = *tuple_members[types_index]; for (int64_t i = 0; - i < GetLeafTypes(type_at_index, pattern.span(), file_table) + i < GetLeafTypes(type_at_index, GetPatternSpan(pattern), + file_table) .size(); ++i) { result.push_back(SomeWildcard()); } types_index += 1; }}, - leaf); + node); } // Check that we got a consistent count between the razed tuple types and the // PatternLeaf vector. - CHECK_EQ(result.size(), GetLeafTypes(type, pattern.span(), file_table).size()) + CHECK_EQ(result.size(), + GetLeafTypes(type, GetPatternSpan(pattern), file_table).size()) << "Sub-pattern leaves and tuple type must be the same size."; return result; } -NdIntervalWithEmpty PatternToInterval(const NameDefTree& pattern, +NdIntervalWithEmpty PatternToInterval(const PatternTree& pattern, const Type& matched_type, absl::Span leaf_types, const TypeInfo& type_info, const ImportData& import_data) { - std::vector pattern_leaves = + std::vector pattern_leaves = ExpandPatternLeaves(pattern, matched_type, type_info.file_table()); CHECK_EQ(pattern_leaves.size(), leaf_types.size()) << "Pattern leaves and leaf types must be the same size."; @@ -363,7 +373,7 @@ NdIntervalWithEmpty PatternToInterval(const NameDefTree& pattern, pattern_leaves[i], *leaf_types[i], type_info, import_data)); } NdIntervalWithEmpty result(intervals); - VLOG(5) << "PatternToInterval; pattern: `" << pattern.ToString() + VLOG(5) << "PatternToInterval; pattern: `" << PatternToString(pattern) << "` type: `" << matched_type.ToString() << "` result: " << result.ToString(/*show_types=*/false); return result; @@ -398,10 +408,11 @@ bool MatchExhaustivenessChecker::IsExhaustive() const { return remaining_.IsEmpty(); } -bool MatchExhaustivenessChecker::AddPattern(const NameDefTree& pattern) { - VLOG(5) << "MatchExhaustivenessChecker::AddPattern: `" << pattern.ToString() - << "` matched_type: `" << matched_type_.ToString() << "` @ " - << pattern.span().ToString(file_table()); +bool MatchExhaustivenessChecker::AddPattern(const PatternTree& pattern) { + VLOG(5) << "MatchExhaustivenessChecker::AddPattern: `" + << PatternToString(pattern) << "` matched_type: `" + << matched_type_.ToString() << "` @ " + << GetPatternSpan(pattern).ToString(file_table()); NdIntervalWithEmpty this_pattern_interval = PatternToInterval( pattern, matched_type_, leaf_types_, type_info_, import_data_); diff --git a/xls/dslx/exhaustiveness/match_exhaustiveness_checker.h b/xls/dslx/exhaustiveness/match_exhaustiveness_checker.h index ae91d37b99..ba93f17a5a 100644 --- a/xls/dslx/exhaustiveness/match_exhaustiveness_checker.h +++ b/xls/dslx/exhaustiveness/match_exhaustiveness_checker.h @@ -41,7 +41,7 @@ class MatchExhaustivenessChecker { // Returns whether we've reached a point of exhaustiveness after incorporating // the given `pattern`. - bool AddPattern(const NameDefTree& pattern); + bool AddPattern(const PatternTree& pattern); // Returns whether, based on already-added patterns, we're exhaustive. bool IsExhaustive() const; diff --git a/xls/dslx/fmt/ast_fmt.cc b/xls/dslx/fmt/ast_fmt.cc index 347183d229..1b93af4891 100644 --- a/xls/dslx/fmt/ast_fmt.cc +++ b/xls/dslx/fmt/ast_fmt.cc @@ -1144,7 +1144,7 @@ DocRef Formatter::FormatColonRef(const ColonRef& n) { arena_.MakeText(StripAnyDotModifier(n.attr()))}); } -DocRef Formatter::FormatForLoopBaseLeader(Keyword keyword, DocRef names_ref, +DocRef Formatter::FormatForLoopBaseLeader(Keyword keyword, DocRef pattern_ref, const ForLoopBase& n, bool is_const_for) { std::vector pieces; @@ -1154,8 +1154,8 @@ DocRef Formatter::FormatForLoopBaseLeader(Keyword keyword, DocRef names_ref, } pieces.push_back(arena_.Make(keyword)); pieces.push_back(arena_.MakeNestIfFlatFits( - /*on_nested_flat_ref=*/names_ref, - /*on_other_ref=*/arena_.MakeConcat(arena_.space(), names_ref))); + /*on_nested_flat_ref=*/pattern_ref, + /*on_other_ref=*/arena_.MakeConcat(arena_.space(), pattern_ref))); if (n.type_annotation() != nullptr) { pieces.push_back(arena_.colon()); @@ -1180,8 +1180,9 @@ DocRef Formatter::FormatForLoopBase(Keyword keyword, const ForLoopBase& n, bool is_const_for) { CHECK(keyword == Keyword::kFor || keyword == Keyword::kUnrollFor) << static_cast>(keyword); - DocRef names_ref = FormatNameDefTree(*n.names()); - DocRef leader = FormatForLoopBaseLeader(keyword, names_ref, n, is_const_for); + DocRef pattern_ref = FormatPatternTree(n.pattern()); + DocRef leader = + FormatForLoopBaseLeader(keyword, pattern_ref, n, is_const_for); std::vector body_pieces; body_pieces.push_back(arena_.hard_line()); @@ -1358,13 +1359,12 @@ DocRef Formatter::FormatInvocation(const Invocation& n) { return result; } -DocRef Formatter::Format(const NameDefTree* n) { return FormatNameDefTree(*n); } - DocRef Formatter::FormatMatchArm(const MatchArm& n) { std::vector pieces; - pieces.push_back(FormatJoin( - n.patterns(), Joiner::kSpaceBarBreak, - [this](const NameDefTree* n) { return Format(n); })); + pieces.push_back(FormatJoin(n.patterns(), Joiner::kSpaceBarBreak, + [this](const PatternTree& pattern) { + return FormatPatternTree(pattern); + })); pieces.push_back(arena_.space()); pieces.push_back(arena_.fat_arrow()); @@ -1894,7 +1894,7 @@ DocRef Formatter::FormatRange(const Range& n) { arena_.break0(), FormatExpr(*n.end())}); } -DocRef Formatter::FormatNameDefTreeLeaf(const NameDefTree::Leaf& n) { +DocRef Formatter::FormatPatternTree(const PatternTree& n) { return absl::visit( Visitor{ [&](const NameDef* n) { return FormatNameDef(*n); }, @@ -1904,29 +1904,16 @@ DocRef Formatter::FormatNameDefTreeLeaf(const NameDefTree::Leaf& n) { [&](const Number* n) { return FormatNumber(*n); }, [&](const ColonRef* n) { return FormatColonRef(*n); }, [&](const Range* n) { return FormatRange(*n); }, + [&](const TuplePattern* n) { return FormatTuplePattern(*n); }, }, n); } -DocRef Formatter::FormatNameDefTree(const NameDefTree& n) { - if (n.is_leaf()) { - return FormatNameDefTreeLeaf(n.leaf()); - } +DocRef Formatter::FormatTuplePattern(const TuplePattern& n) { std::vector pieces = {arena_.oparen()}; - std::vector> flattened = - n.Flatten1(); - for (size_t i = 0; i < flattened.size(); ++i) { - const auto& item = flattened[i]; - absl::visit(Visitor{ - [&](const NameDefTree::Leaf& leaf) { - pieces.push_back(FormatNameDefTreeLeaf(leaf)); - }, - [&](const NameDefTree* subtree) { - pieces.push_back(FormatNameDefTree(*subtree)); - }, - }, - item); - if (i + 1 != flattened.size()) { + for (size_t i = 0; i < n.members().size(); ++i) { + pieces.push_back(FormatPatternTree(n.members()[i])); + if (i + 1 != n.members().size()) { pieces.push_back(arena_.comma()); pieces.push_back(arena_.break1()); } @@ -2011,15 +1998,15 @@ DocRef Formatter::FormatBlockedExprLeader(const Expr& e) { } case AstNodeKind::kFor: { const ForLoopBase& n = static_cast(e); - DocRef names_ref = FormatNameDefTree(*n.names()); - return FormatForLoopBaseLeader(Keyword::kFor, names_ref, n, + DocRef pattern_ref = FormatPatternTree(n.pattern()); + return FormatForLoopBaseLeader(Keyword::kFor, pattern_ref, n, /*is_const_for=*/false); } case AstNodeKind::kConstFor: { const ConstFor& n = static_cast(e); Keyword keyword = n.IsUnrollFor() ? Keyword::kUnrollFor : Keyword::kFor; - DocRef names_ref = FormatNameDefTree(*n.names()); - return FormatForLoopBaseLeader(keyword, names_ref, n, !n.IsUnrollFor()); + DocRef pattern_ref = FormatPatternTree(n.pattern()); + return FormatForLoopBaseLeader(keyword, pattern_ref, n, !n.IsUnrollFor()); } default: LOG(FATAL) << "Unhandled node kind for FmtBlockedExprLeader: `" @@ -3077,7 +3064,7 @@ DocRef Formatter::FormatUse(const Use& n) { DocRef Formatter::FormatLet(const Let& n, bool trailing_semi) { std::vector leader_pieces = { arena_.Make(n.is_const() ? Keyword::kConst : Keyword::kLet), - arena_.space(), FormatNameDefTree(*n.name_def_tree())}; + arena_.space(), FormatPatternTree(n.pattern())}; if (const TypeAnnotation* t = n.type_annotation()) { leader_pieces.push_back(arena_.colon()); leader_pieces.push_back(arena_.space()); @@ -3088,7 +3075,7 @@ DocRef Formatter::FormatLet(const Let& n, bool trailing_semi) { leader_pieces.push_back(arena_.equals()); Pos lhs_limit = n.type_annotation() != nullptr ? n.type_annotation()->span().limit() - : n.name_def_tree()->span().limit(); + : GetPatternSpan(n.pattern()).limit(); std::optional comments_doc = FormatCommentsNested(lhs_limit, n.rhs()->span().start()); diff --git a/xls/dslx/fmt/ast_fmt.h b/xls/dslx/fmt/ast_fmt.h index 6bc10c98be..2a41e2af53 100644 --- a/xls/dslx/fmt/ast_fmt.h +++ b/xls/dslx/fmt/ast_fmt.h @@ -145,13 +145,12 @@ class Formatter { virtual DocRef FormatMatch(const Match& n); virtual DocRef FormatModuleMember(const ModuleMember& n); virtual DocRef FormatNameDef(const NameDef& n); - virtual DocRef FormatNameDefTree(const NameDefTree& n); - virtual DocRef FormatNameDefTreeLeaf(const NameDefTree::Leaf& n); virtual DocRef FormatNameRef(const NameRef& n); virtual DocRef FormatNumber(const Number& n); virtual DocRef FormatParametricBinding(const ParametricBinding& n); virtual DocRef FormatParametricBindingPtr(const ParametricBinding* n); virtual DocRef FormatParams(absl::Span params); + virtual DocRef FormatPatternTree(const PatternTree& n); virtual DocRef FormatProc(const Proc& n, bool is_test = false); virtual DocRef FormatProcAlias(const ProcAlias& n); virtual DocRef FormatProcDef(const ProcDef& n); @@ -173,6 +172,7 @@ class Formatter { virtual DocRef FormatTestProc(const TestProc& n); virtual DocRef FormatTrait(const Trait& n); virtual DocRef FormatTupleIndex(const TupleIndex& n); + virtual DocRef FormatTuplePattern(const TuplePattern& n); virtual DocRef FormatTupleTypeAnnotation(const TupleTypeAnnotation& n); virtual DocRef FormatTypeAlias(const TypeAlias& n); virtual DocRef FormatTypeRef(const TypeRef& n); @@ -189,7 +189,6 @@ class Formatter { DocRef Format(const Expr* n); DocRef Format(const TypeAnnotation* n); - DocRef Format(const NameDefTree* n); DocRef FormatBreakBody(const Array& n); DocRef FormatBreakRest(const StructInstance& n); Pos FormatCollectInlineComments(const Pos& prev_limit, @@ -205,7 +204,7 @@ class Formatter { DocRef FormatFlatRest(const StructInstance& n); DocRef FormatForLoopBase(Keyword keyword, const ForLoopBase& n, bool is_const_for); - DocRef FormatForLoopBaseLeader(Keyword keyword, DocRef names_ref, + DocRef FormatForLoopBaseLeader(Keyword keyword, DocRef pattern_ref, const ForLoopBase& n, bool is_const_for); DocRef FormatJoinWithAttr(std::optional attr, DocRef rest); DocRef FormatJoinWithAttrs(absl::Span attrs, DocRef rest); diff --git a/xls/dslx/frontend/ast.cc b/xls/dslx/frontend/ast.cc index 9e79f2dc9e..6a82cdc305 100644 --- a/xls/dslx/frontend/ast.cc +++ b/xls/dslx/frontend/ast.cc @@ -289,8 +289,8 @@ std::string_view AstNodeKindToString(AstNodeKind kind) { return "struct instance"; case AstNodeKind::kSplatStructInstance: return "splat struct instance"; - case AstNodeKind::kNameDefTree: - return "name definition tree"; + case AstNodeKind::kTuplePattern: + return "tuple pattern"; case AstNodeKind::kIndex: return "index"; case AstNodeKind::kRange: @@ -851,19 +851,19 @@ std::vector ParametricBinding::GetChildren(bool want_types) const { std::string MatchArm::ToString() const { std::string patterns_or = absl::StrJoin( - patterns_, " | ", [](std::string* out, NameDefTree* name_def_tree) { - absl::StrAppend(out, name_def_tree->ToString()); + patterns_, " | ", [](std::string* out, const PatternTree& pattern) { + absl::StrAppend(out, PatternToString(pattern)); }); return absl::StrFormat("%s => %s", patterns_or, expr_->ToString()); } For::~For() = default; -ConstFor::ConstFor(Module* owner, Span span, NameDefTree* names, +ConstFor::ConstFor(Module* owner, Span span, PatternTree pattern, TypeAnnotation* type_annotation, Expr* iterable, StatementBlock* body, Expr* init, bool is_unroll_for, bool in_parens) - : ForLoopBase(owner, span, names, type_annotation, iterable, body, init, + : ForLoopBase(owner, span, pattern, type_annotation, iterable, body, init, in_parens), is_unroll_for_(is_unroll_for) {} @@ -1398,8 +1398,8 @@ bool IsConstant(AstNode* node) { std::vector MatchArm::GetChildren(bool want_types) const { std::vector results; results.reserve(patterns_.size()); - for (NameDefTree* ndt : patterns_) { - results.push_back(ndt); + for (const PatternTree& pattern : patterns_) { + results.push_back(ToAstNode(pattern)); } results.push_back(expr_); return results; @@ -2320,18 +2320,18 @@ std::string StatementBlock::ToStringInternal() const { // -- class ForLoopBase -ForLoopBase::ForLoopBase(Module* owner, Span span, NameDefTree* names, +ForLoopBase::ForLoopBase(Module* owner, Span span, PatternTree pattern, TypeAnnotation* type_annotation, Expr* iterable, StatementBlock* body, Expr* init, bool in_parens) : Expr(owner, span, in_parens), - names_(names), + pattern_(pattern), type_annotation_(type_annotation), iterable_(iterable), body_(body), init_(init) {} std::vector ForLoopBase::GetChildren(bool want_types) const { - std::vector results = {names_}; + std::vector results = {ToAstNode(pattern_)}; if (want_types && type_annotation_ != nullptr) { results.push_back(type_annotation_); } @@ -2346,9 +2346,9 @@ std::string ForLoopBase::ToStringInternal() const { if (type_annotation_ != nullptr) { type_str = absl::StrCat(": ", type_annotation_->ToString()); } - return absl::StrFormat("%s %s%s in %s %s(%s)", keyword(), names_->ToString(), - type_str, iterable_->ToString(), body_->ToString(), - init_->ToString()); + return absl::StrFormat( + "%s %s%s in %s %s(%s)", keyword(), PatternToString(pattern_), type_str, + iterable_->ToString(), body_->ToString(), init_->ToString()); } // -- class Function @@ -2567,7 +2567,7 @@ std::string Lambda::ToStringInternal() const { // -- class MatchArm -MatchArm::MatchArm(Module* owner, Span span, std::vector patterns, +MatchArm::MatchArm(Module* owner, Span span, std::vector patterns, Expr* expr) : AstNode(owner), span_(std::move(span)), @@ -2579,7 +2579,8 @@ MatchArm::MatchArm(Module* owner, Span span, std::vector patterns, MatchArm::~MatchArm() = default; Span MatchArm::GetPatternSpan() const { - return Span(patterns_[0]->span().start(), patterns_.back()->span().limit()); + return Span(xls::dslx::GetPatternSpan(patterns_[0]).start(), + xls::dslx::GetPatternSpan(patterns_.back()).limit()); } // -- class NameRef @@ -2835,75 +2836,236 @@ std::string XlsTuple::ToStringInternal() const { return result; } -// -- class NameDefTree +// -- class TuplePattern -NameDefTree::~NameDefTree() = default; +TuplePattern::~TuplePattern() = default; -std::vector NameDefTree::GetChildren(bool want_types) const { - if (std::holds_alternative(tree_)) { - return {ToAstNode(std::get(tree_))}; +std::vector TuplePattern::GetChildren(bool want_types) const { + std::vector result; + result.reserve(members_.size()); + for (const PatternTree& member : members_) { + result.push_back(ToAstNode(member)); } - const Nodes& nodes = std::get(tree_); - return ToAstNodes(nodes); + return result; } -std::string NameDefTree::ToString() const { - if (is_leaf()) { - return ToAstNode(leaf())->ToString(); - } - - std::string guts = - absl::StrJoin(nodes(), ", ", [](std::string* out, NameDefTree* node) { - absl::StrAppend(out, node->ToString()); +std::string TuplePattern::ToString() const { + std::string guts = absl::StrJoin( + members_, ", ", [](std::string* out, const PatternTree& member) { + absl::StrAppend(out, PatternToString(member)); }); return absl::StrFormat("(%s)", guts); } -std::vector NameDefTree::Flatten() const { - if (is_leaf()) { - return {leaf()}; +namespace { + +PatternLeaf PatternTreeToLeaf(const PatternTree& pattern) { + return absl::visit( + Visitor{[](NameDef* node) -> PatternLeaf { return node; }, + [](NameRef* node) -> PatternLeaf { return node; }, + [](WildcardPattern* node) -> PatternLeaf { return node; }, + [](Number* node) -> PatternLeaf { return node; }, + [](ColonRef* node) -> PatternLeaf { return node; }, + [](Range* node) -> PatternLeaf { return node; }, + [](RestOfTuple* node) -> PatternLeaf { return node; }, + [](TuplePattern* /*node*/) -> PatternLeaf { + LOG(FATAL) << "Tuple pattern is not a pattern leaf"; + return static_cast(nullptr); + }}, + pattern); +} + +ConstPatternLeaf PatternTreeToLeaf(const ConstPatternTree& pattern) { + return absl::visit( + Visitor{ + [](const NameDef* node) -> ConstPatternLeaf { return node; }, + [](const NameRef* node) -> ConstPatternLeaf { return node; }, + [](const WildcardPattern* node) -> ConstPatternLeaf { return node; }, + [](const Number* node) -> ConstPatternLeaf { return node; }, + [](const ColonRef* node) -> ConstPatternLeaf { return node; }, + [](const Range* node) -> ConstPatternLeaf { return node; }, + [](const RestOfTuple* node) -> ConstPatternLeaf { return node; }, + [](const TuplePattern* /*node*/) -> ConstPatternLeaf { + LOG(FATAL) << "Tuple pattern is not a pattern leaf"; + return static_cast(nullptr); + }}, + pattern); +} + +} // namespace + +AstNode* ToAstNode(const PatternTree& pattern) { + return absl::visit([](auto* node) -> AstNode* { return node; }, pattern); +} + +const AstNode* ToAstNode(const ConstPatternTree& pattern) { + return absl::visit([](const auto* node) -> const AstNode* { return node; }, + pattern); +} + +ConstPatternTree ToConstPatternTree(const PatternTree& pattern) { + return absl::visit([](auto* node) -> ConstPatternTree { return node; }, + pattern); +} + +const Span& GetPatternSpan(const PatternTree& pattern) { + return absl::visit([](auto* node) -> const Span& { return node->span(); }, + pattern); +} + +const Span& GetPatternSpan(const ConstPatternTree& pattern) { + return absl::visit( + [](const auto* node) -> const Span& { return node->span(); }, pattern); +} + +std::string PatternToString(const PatternTree& pattern) { + return ToAstNode(pattern)->ToString(); +} + +std::string PatternToString(const ConstPatternTree& pattern) { + return ToAstNode(pattern)->ToString(); +} + +std::vector FlattenPattern(const PatternTree& pattern) { + if (!std::holds_alternative(pattern)) { + return {PatternTreeToLeaf(pattern)}; } - std::vector results; - for (const NameDefTree* node : std::get(tree_)) { - auto node_leaves = node->Flatten(); - results.insert(results.end(), node_leaves.begin(), node_leaves.end()); + std::vector result; + for (const PatternTree& member : + std::get(pattern)->members()) { + std::vector member_leaves = FlattenPattern(member); + result.insert(result.end(), member_leaves.begin(), member_leaves.end()); } - return results; + return result; } -std::vector NameDefTree::GetNameDefs() const { - std::vector results; - for (Leaf leaf : Flatten()) { +std::vector FlattenPattern(const ConstPatternTree& pattern) { + if (!std::holds_alternative(pattern)) { + return {PatternTreeToLeaf(pattern)}; + } + std::vector result; + for (const PatternTree& member : + std::get(pattern)->members()) { + std::vector member_leaves = + FlattenPattern(ToConstPatternTree(member)); + result.insert(result.end(), member_leaves.begin(), member_leaves.end()); + } + return result; +} + +std::vector FlattenPattern1(const PatternTree& pattern) { + if (!std::holds_alternative(pattern)) { + return {pattern}; + } + return std::get(pattern)->members(); +} + +std::vector FlattenPattern1(const ConstPatternTree& pattern) { + if (!std::holds_alternative(pattern)) { + return {pattern}; + } + std::vector result; + for (const PatternTree& member : + std::get(pattern)->members()) { + result.push_back(ToConstPatternTree(member)); + } + return result; +} + +std::vector GetPatternNameDefs(const PatternTree& pattern) { + std::vector result; + for (const PatternLeaf& leaf : FlattenPattern(pattern)) { if (std::holds_alternative(leaf)) { - results.push_back(std::get(leaf)); + result.push_back(std::get(leaf)); } } - return results; + return result; } -std::vector> -NameDefTree::Flatten1() const { - if (is_leaf()) { - return {leaf()}; - } - std::vector> result; - for (NameDefTree* ndt : nodes()) { - if (ndt->is_leaf()) { - result.push_back(ndt->leaf()); - } else { - result.push_back(ndt); +std::vector GetPatternNameDefs( + const ConstPatternTree& pattern) { + std::vector result; + for (const ConstPatternLeaf& leaf : FlattenPattern(pattern)) { + if (std::holds_alternative(leaf)) { + result.push_back(std::get(leaf)); } } return result; } +bool IsIrrefutablePattern(const PatternTree& pattern) { + std::vector leaves = FlattenPattern(pattern); + return std::all_of(leaves.begin(), leaves.end(), [](PatternLeaf leaf) { + return std::holds_alternative(leaf) || + std::holds_alternative(leaf); + }); +} + +bool IsIrrefutablePattern(const ConstPatternTree& pattern) { + std::vector leaves = FlattenPattern(pattern); + return std::all_of(leaves.begin(), leaves.end(), [](ConstPatternLeaf leaf) { + return std::holds_alternative(leaf) || + std::holds_alternative(leaf); + }); +} + +bool IsWildcardLeaf(const PatternTree& pattern) { + return std::holds_alternative(pattern); +} + +bool IsWildcardLeaf(const ConstPatternTree& pattern) { + return std::holds_alternative(pattern); +} + +bool IsRestOfTupleLeaf(const PatternTree& pattern) { + return std::holds_alternative(pattern); +} + +bool IsRestOfTupleLeaf(const ConstPatternTree& pattern) { + return std::holds_alternative(pattern); +} + +absl::Status DoPatternPreorder( + const PatternTree& pattern, + const std::function& f, + int64_t level) { + if (!std::holds_alternative(pattern)) { + return absl::OkStatus(); + } + const std::vector& members = + std::get(pattern)->members(); + for (int64_t i = 0; i < members.size(); ++i) { + XLS_RETURN_IF_ERROR(f(members[i], level, i)); + XLS_RETURN_IF_ERROR(DoPatternPreorder(members[i], f, level + 1)); + } + return absl::OkStatus(); +} + +absl::Status DoPatternPreorder( + const ConstPatternTree& pattern, + const std::function& f, + int64_t level) { + if (!std::holds_alternative(pattern)) { + return absl::OkStatus(); + } + const std::vector& members = + std::get(pattern)->members(); + for (int64_t i = 0; i < members.size(); ++i) { + ConstPatternTree member = ToConstPatternTree(members[i]); + XLS_RETURN_IF_ERROR(f(member, level, i)); + XLS_RETURN_IF_ERROR(DoPatternPreorder(member, f, level + 1)); + } + return absl::OkStatus(); +} + // -- class Let -Let::Let(Module* owner, Span span, NameDefTree* name_def_tree, +Let::Let(Module* owner, Span span, PatternTree pattern, TypeAnnotation* type_annotation, Expr* rhs, bool is_const) : AstNode(owner), span_(std::move(span)), - name_def_tree_(name_def_tree), + pattern_(pattern), type_annotation_(type_annotation), rhs_(rhs), is_const_(is_const) {} @@ -2911,7 +3073,7 @@ Let::Let(Module* owner, Span span, NameDefTree* name_def_tree, Let::~Let() = default; std::vector Let::GetChildren(bool want_types) const { - std::vector results = {name_def_tree_}; + std::vector results = {ToAstNode(pattern_)}; if (type_annotation_ != nullptr && want_types) { results.push_back(type_annotation_); } @@ -2930,7 +3092,7 @@ std::string Let::ToString() const { ? absl::StrCat(" =", rhs_str) : absl::StrCat(" = ", rhs_str); return absl::StrFormat("%s %s%s%s;", is_const_ ? "const" : "let", - name_def_tree_->ToString(), type_str, eq_and_rhs); + PatternToString(pattern_), type_str, eq_and_rhs); } // -- class Expr diff --git a/xls/dslx/frontend/ast.h b/xls/dslx/frontend/ast.h index 0156a77395..991650ef59 100644 --- a/xls/dslx/frontend/ast.h +++ b/xls/dslx/frontend/ast.h @@ -50,7 +50,7 @@ // Higher-order macro for all the Expr node leaf types (non-abstract). #define XLS_DSLX_EXPR_NODE_EACH(X) \ - /* keep-sorted start */ \ + /* keep-sorted start */ \ X(AllOnesMacro) \ X(Array) \ X(Attr) \ @@ -87,7 +87,7 @@ // (Note that this includes all the Expr node leaf kinds listed in // XLS_DSLX_EXPR_NODE_EACH). #define XLS_DSLX_AST_NODE_EACH(X) \ - /* keep-sorted start */ \ + /* keep-sorted start */ \ X(Attribute) \ X(BuiltinNameDef) \ X(ConstAssert) \ @@ -101,7 +101,6 @@ X(MatchArm) \ X(Module) \ X(NameDef) \ - X(NameDefTree) \ X(Param) \ X(ParametricBinding) \ X(Proc) \ @@ -117,15 +116,16 @@ X(TestFunction) \ X(TestProc) \ X(Trait) \ + X(TuplePattern) \ X(TypeAlias) \ X(TypeRef) \ X(Use) \ X(UseTreeEntry) \ X(WidthSlice) \ X(WildcardPattern) \ - /* keep-sorted end */ \ + /* keep-sorted end */ \ /* type annotations */ \ - /* keep-sorted start */ \ + /* keep-sorted start */ \ X(AnyTypeAnnotation) \ X(ArrayTypeAnnotation) \ X(BuiltinTypeAnnotation) \ @@ -143,7 +143,7 @@ X(TupleTypeAnnotation) \ X(TypeRefTypeAnnotation) \ X(TypeVariableTypeAnnotation) \ - /* keep-sorted end */ \ + /* keep-sorted end */ \ XLS_DSLX_EXPR_NODE_EACH(X) namespace xls::dslx { @@ -193,6 +193,26 @@ class TypeAnnotation; using ExprOrType = std::variant; Span ExprOrTypeSpan(const ExprOrType& expr_or_type); +// A pattern is represented directly by its concrete syntax node. Parentheses +// are the only recursive shape in the current language, represented by +// TuplePattern below. +using PatternLeaf = std::variant; +using PatternTree = + std::variant; +// Const pattern views preserve read-only capability when traversed through the +// PatternTree helper APIs. PatternTree itself remains mutable for AST +// construction and lowering paths. +using ConstPatternLeaf = + std::variant; +using ConstPatternTree = + std::variant; + // Name definitions can be either built in (BuiltinNameDef, in which case they // have no effective position) or defined in the user AST (NameDef). using AnyNameDef = std::variant; @@ -2664,7 +2684,7 @@ class Function : public AstNode { // span: The span of the match arm (both matcher and expr). class MatchArm : public AstNode { public: - MatchArm(Module* owner, Span span, std::vector patterns, + MatchArm(Module* owner, Span span, std::vector patterns, Expr* expr); ~MatchArm() override; @@ -2679,7 +2699,7 @@ class MatchArm : public AstNode { std::vector GetChildren(bool want_types) const override; - const std::vector& patterns() const { return patterns_; } + const std::vector& patterns() const { return patterns_; } Expr* expr() const { return expr_; } const Span& span() const { return span_; } std::optional GetSpan() const override { return span_; } @@ -2690,7 +2710,7 @@ class MatchArm : public AstNode { private: Span span_; - std::vector patterns_; // Note: never empty. + std::vector patterns_; // Note: never empty. Expr* expr_; // Expression that is executed if one of the patterns matches. }; @@ -4179,7 +4199,7 @@ class XlsTuple : public Expr { // types of loops should be as much the same as possible. class ForLoopBase : public Expr { public: - ForLoopBase(Module* owner, Span span, NameDefTree* names, + ForLoopBase(Module* owner, Span span, PatternTree pattern, TypeAnnotation* type, Expr* iterable, StatementBlock* body, Expr* init, bool in_parens = false); @@ -4188,10 +4208,10 @@ class ForLoopBase : public Expr { std::vector GetChildren(bool want_types) const override; - // Names bound in the body of the loop. - NameDefTree* names() const { return names_; } + // Pattern matched against each iterator/accumulator pair. + const PatternTree& pattern() const { return pattern_; } - // Annotation corresponding to "names". + // Annotation corresponding to the pattern. TypeAnnotation* type_annotation() const { return type_annotation_; } // Expression for "thing to iterate over". @@ -4214,7 +4234,7 @@ class ForLoopBase : public Expr { private: std::string ToStringInternal() const override; - NameDefTree* names_; + PatternTree pattern_; TypeAnnotation* type_annotation_; Expr* iterable_; StatementBlock* body_; @@ -4247,7 +4267,7 @@ class For : public ForLoopBase { // number of elements in the iterable. class ConstFor : public ForLoopBase { public: - ConstFor(Module* owner, Span span, NameDefTree* names, TypeAnnotation* type, + ConstFor(Module* owner, Span span, PatternTree pattern, TypeAnnotation* type, Expr* iterable, StatementBlock* body, Expr* init, bool is_unroll_for, bool in_parens = false); @@ -4372,130 +4392,83 @@ class ConstantDef : public AstNode { bool is_public_; }; -// Tree of name definition nodes; e.g. -// -// in LHS of let bindings. -// -// For example: -// -// let (a, (b, (c)), d) = ... -// -// Makes a: -// -// NameDefTree((NameDef('a'), -// NameDefTree(( -// NameDef('b'), -// NameDefTree(( -// NameDef('c'))))), -// NameDef('d'))) -// -// A "NameDef" is an AST node that signifies an identifier is being bound, so -// this is simply a tree of those (with the tree being constructed via tuples; -// leaves are NameDefs, interior nodes are tuples). +// Represents parenthesized pattern syntax such as (a, (b, c)). // -// Attributes: -// span: The span of the names at this level of the tree. -// tree: The subtree this represents (either a tuple of subtrees or a leaf). -class NameDefTree : public AstNode { +// Leaves are stored directly in PatternTree; this node is the only recursive +// pattern shape in the current language. +class TuplePattern : public AstNode { public: - using Nodes = std::vector; - using Leaf = std::variant; - - NameDefTree(Module* owner, Span span, std::variant tree) - : AstNode(owner), span_(std::move(span)), tree_(std::move(tree)) {} + TuplePattern(Module* owner, Span span, std::vector members) + : AstNode(owner), span_(std::move(span)), members_(std::move(members)) {} - ~NameDefTree() override; + ~TuplePattern() override; - AstNodeKind kind() const override { return AstNodeKind::kNameDefTree; } + AstNodeKind kind() const override { return AstNodeKind::kTuplePattern; } absl::Status Accept(AstNodeVisitor* v) const override { - return v->HandleNameDefTree(this); + return v->HandleTuplePattern(this); } - std::string_view GetNodeTypeName() const override { return "NameDefTree"; } + std::string_view GetNodeTypeName() const override { return "TuplePattern"; } std::string ToString() const override; std::vector GetChildren(bool want_types) const override; - bool is_leaf() const { return std::holds_alternative(tree_); } - Leaf leaf() const { return std::get(tree_); } - - const Nodes& nodes() const { return std::get(tree_); } - - // Flattens this NameDefTree a single level, unwrapping any leaf NDTs; e.g. - // - // LEAF:a => [LEAF:a] - // LEAF:a, NODES:[NDT:b, NDT:c], LEAF:d => [LEAF:a, NDT:b, NDT:c, LEAF:d] - // NODES:[NDT:a, NDT:LEAF:c], NODES[NDT:c] => [NDT:a, NDT:b, LEAF:c, NDT:d] - // - // This is useful for flattening a tuple a single level; e.g. where a - // NameDefTree is going to be used as variadic args in for-loop to function - // conversion. - std::vector> Flatten1() const; - - // Flattens the (recursive) NameDefTree into a list of leaves. - std::vector Flatten() const; - - // Filters the values from Flatten() to just NameDef leaves. - std::vector GetNameDefs() const; - - // A pattern is irrefutable if it always causes a successful match. - // - // Returns whether this NameDefTree is known-irrefutable. - bool IsIrrefutable() const { - auto leaves = Flatten(); - return std::all_of(leaves.begin(), leaves.end(), [](Leaf leaf) { - return std::holds_alternative(leaf) || - std::holds_alternative(leaf); - }); - } - - bool IsWildcardLeaf() const { - return is_leaf() && std::holds_alternative(leaf()); - } - - bool IsRestOfTupleLeaf() const { - return is_leaf() && std::holds_alternative(leaf()); - } - - // Performs a preorder traversal under this node in the NameDefTree. - // - // Args: - // f: Callback invoked as `f(NameDefTree*, level, branchno)`. - // level: Current level of the node. - absl::Status DoPreorder( - const std::function& f, - int64_t level = 1) { - if (is_leaf()) { - return absl::OkStatus(); - } - for (int64_t i = 0; i < nodes().size(); ++i) { - NameDefTree* node = nodes()[i]; - XLS_RETURN_IF_ERROR(f(node, level, i)); - XLS_RETURN_IF_ERROR(node->DoPreorder(f, level + 1)); - } - return absl::OkStatus(); - } - - [[maybe_unused]] const std::variant& tree() const { - return tree_; - } + const std::vector& members() const { return members_; } const Span& span() const { return span_; } std::optional GetSpan() const override { return span_; } private: Span span_; - std::variant tree_; + std::vector members_; }; +AstNode* ToAstNode(const PatternTree& pattern); +const AstNode* ToAstNode(const ConstPatternTree& pattern); +ConstPatternTree ToConstPatternTree(const PatternTree& pattern); +const Span& GetPatternSpan(const PatternTree& pattern); +const Span& GetPatternSpan(const ConstPatternTree& pattern); +std::string PatternToString(const PatternTree& pattern); +std::string PatternToString(const ConstPatternTree& pattern); + +// FlattenPattern returns all leaves in source order. FlattenPattern1 unwraps +// only the root tuple; nested tuples remain TuplePattern* alternatives. +std::vector FlattenPattern(const PatternTree& pattern); +std::vector FlattenPattern(const ConstPatternTree& pattern); +std::vector FlattenPattern1(const PatternTree& pattern); +std::vector FlattenPattern1(const ConstPatternTree& pattern); + +// These helpers centralize recursive pattern traversal. +std::vector GetPatternNameDefs(const PatternTree& pattern); +std::vector GetPatternNameDefs(const ConstPatternTree& pattern); +bool IsIrrefutablePattern(const PatternTree& pattern); +bool IsIrrefutablePattern(const ConstPatternTree& pattern); + +// These predicates inspect only the root alternative. +bool IsWildcardLeaf(const PatternTree& pattern); +bool IsWildcardLeaf(const ConstPatternTree& pattern); +bool IsRestOfTupleLeaf(const PatternTree& pattern); +bool IsRestOfTupleLeaf(const ConstPatternTree& pattern); + +// Visits each member below a tuple root in preorder. The root itself is not +// passed to f; level is one-based and index is local to the member's tuple. +absl::Status DoPatternPreorder( + const PatternTree& pattern, + const std::function& f, + int64_t level = 1); +absl::Status DoPatternPreorder( + const ConstPatternTree& pattern, + const std::function& f, + int64_t level = 1); + // Represents a let-binding expression. class Let : public AstNode { public: // A Let's body can be nullopt if it's the last expr // in an unroll_for or a const for body. - Let(Module* owner, Span span, NameDefTree* name_def_tree, - TypeAnnotation* type, Expr* rhs, bool is_const); + Let(Module* owner, Span span, PatternTree pattern, TypeAnnotation* type, + Expr* rhs, bool is_const); ~Let() override; @@ -4512,7 +4485,7 @@ class Let : public AstNode { std::vector GetChildren(bool want_types) const override; - NameDefTree* name_def_tree() const { return name_def_tree_; } + const PatternTree& pattern() const { return pattern_; } TypeAnnotation* type_annotation() const { return type_annotation_; } Expr* rhs() const { return rhs_; } bool is_const() const { return is_const_; } @@ -4525,8 +4498,8 @@ class Let : public AstNode { // let (a, b, (c)) = (1, 2, (3,)); // ... // - // the name_def_tree is `(a, b, (c))` - NameDefTree* name_def_tree_; + // the pattern is (a, b, (c)) + PatternTree pattern_; // The optional annotated type on the let expression, may be null. TypeAnnotation* type_annotation_; diff --git a/xls/dslx/frontend/ast_cloner.cc b/xls/dslx/frontend/ast_cloner.cc index 830b39500d..d6748aaa48 100644 --- a/xls/dslx/frontend/ast_cloner.cc +++ b/xls/dslx/frontend/ast_cloner.cc @@ -54,6 +54,14 @@ class AstCloner : public AstNodeVisitor { return module_.has_value() ? *module_ : n->owner(); } + PatternTree ClonePattern(const PatternTree& pattern) const { + return absl::visit( + [&](auto* node) -> PatternTree { + return absl::down_cast(old_to_new_.at(node)); + }, + pattern); + } + absl::Status HandleArray(const Array* n) override { XLS_RETURN_IF_ERROR(VisitChildren(n)); @@ -349,7 +357,7 @@ class AstCloner : public AstNodeVisitor { absl::Status HandleFor(const For* n) override { XLS_RETURN_IF_ERROR(VisitChildren(n)); - XLS_RETURN_IF_ERROR(ReplaceOrVisit(n->names())); + XLS_RETURN_IF_ERROR(ReplaceOrVisit(ToAstNode(n->pattern()))); XLS_RETURN_IF_ERROR(ReplaceOrVisit(n->type_annotation())); XLS_RETURN_IF_ERROR(ReplaceOrVisit(n->iterable())); XLS_RETURN_IF_ERROR(ReplaceOrVisit(n->body())); @@ -361,8 +369,7 @@ class AstCloner : public AstNodeVisitor { old_to_new_.at(n->type_annotation())); old_to_new_[n] = module(n)->Make( - n->span(), absl::down_cast(old_to_new_.at(n->names())), - new_type_annotation, + n->span(), ClonePattern(n->pattern()), new_type_annotation, absl::down_cast(old_to_new_.at(n->iterable())), absl::down_cast(old_to_new_.at(n->body())), absl::down_cast(old_to_new_.at(n->init()))); @@ -577,10 +584,8 @@ class AstCloner : public AstNodeVisitor { } old_to_new_[n] = module(n)->Make( - n->span(), - absl::down_cast(old_to_new_.at(n->name_def_tree())), - new_type, absl::down_cast(old_to_new_.at(n->rhs())), - n->is_const()); + n->span(), ClonePattern(n->pattern()), new_type, + absl::down_cast(old_to_new_.at(n->rhs())), n->is_const()); return absl::OkStatus(); } @@ -602,16 +607,21 @@ class AstCloner : public AstNodeVisitor { absl::Status HandleMatchArm(const MatchArm* n) override { XLS_RETURN_IF_ERROR(VisitChildren(n)); - std::vector new_patterns; + std::vector new_patterns; new_patterns.reserve(n->patterns().size()); - for (const NameDefTree* pattern : n->patterns()) { - new_patterns.push_back( - absl::down_cast(old_to_new_.at(pattern))); + for (const PatternTree& pattern : n->patterns()) { + new_patterns.push_back(ClonePattern(pattern)); } - old_to_new_[n] = module(n)->Make( + MatchArm* new_arm = module(n)->Make( n->span(), new_patterns, absl::down_cast(old_to_new_.at(n->expr()))); + for (const PatternTree& pattern : new_arm->patterns()) { + for (NameDef* name_def : GetPatternNameDefs(pattern)) { + name_def->set_definer(new_arm); + } + } + old_to_new_[n] = new_arm; return absl::OkStatus(); } @@ -664,26 +674,16 @@ class AstCloner : public AstNodeVisitor { return absl::OkStatus(); } - absl::Status HandleNameDefTree(const NameDefTree* n) override { + absl::Status HandleTuplePattern(const TuplePattern* n) override { XLS_RETURN_IF_ERROR(VisitChildren(n)); - if (n->is_leaf()) { - NameDefTree::Leaf leaf; - absl::visit( - [&](auto* x) { - leaf = absl::down_cast(old_to_new_.at(x)); - }, - n->leaf()); - old_to_new_[n] = module(n)->Make(n->span(), leaf); - return absl::OkStatus(); - } - - NameDefTree::Nodes nodes; - nodes.reserve(n->nodes().size()); - for (const auto& node : n->nodes()) { - nodes.push_back(absl::down_cast(old_to_new_.at(node))); + std::vector members; + members.reserve(n->members().size()); + for (const PatternTree& member : n->members()) { + members.push_back(ClonePattern(member)); } - old_to_new_[n] = module(n)->Make(n->span(), nodes); + old_to_new_[n] = + module(n)->Make(n->span(), std::move(members)); return absl::OkStatus(); } @@ -1308,8 +1308,7 @@ class AstCloner : public AstNodeVisitor { old_to_new_.at(n->type_annotation())); old_to_new_[n] = module(n)->Make( - n->span(), absl::down_cast(old_to_new_.at(n->names())), - new_type_annotation, + n->span(), ClonePattern(n->pattern()), new_type_annotation, absl::down_cast(old_to_new_.at(n->iterable())), absl::down_cast(old_to_new_.at(n->body())), absl::down_cast(old_to_new_.at(n->init())), n->IsUnrollFor(), diff --git a/xls/dslx/frontend/ast_cloner_test.cc b/xls/dslx/frontend/ast_cloner_test.cc index b50bbec445..a7c1d551a6 100644 --- a/xls/dslx/frontend/ast_cloner_test.cc +++ b/xls/dslx/frontend/ast_cloner_test.cc @@ -24,13 +24,13 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/base/casts.h" #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/status/status.h" #include "absl/status/statusor.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" #include "xls/common/status/matchers.h" #include "xls/common/status/ret_check.h" #include "xls/common/status/status_macros.h" @@ -1369,7 +1369,7 @@ TEST(AstClonerTest, ReplacerUsesOldToNewMappingForNameRef) { if (auto let_ptr = std::get_if(&const_cast(wrapped)); let_ptr != nullptr && *let_ptr != nullptr) { - auto name_defs = (*let_ptr)->name_def_tree()->GetNameDefs(); + auto name_defs = GetPatternNameDefs((*let_ptr)->pattern()); if (!name_defs.empty() && name_defs[0]->identifier() == "b") { b_name_def = name_defs[0]; break; @@ -1673,7 +1673,7 @@ TEST(AstClonerTest, FormatMacroWithVerbosity) { } TEST(AstClonerTest, Match) { - // Try to every potential NameDefTree Leaf type (NameRef, NameDef, + // Try every potential PatternTree leaf type (NameRef, NameDef, // WildcardPattern, Number, ColonRef). constexpr std::string_view kProgram = R"(import foo; fn main(x: u32, y: u32) -> u32 { @@ -1693,6 +1693,56 @@ fn main(x: u32, y: u32) -> u32 { EXPECT_EQ(kProgram, clone->ToString()); } +TEST(AstClonerTest, PatternTreeShapeAndMatchDefiners) { + constexpr std::string_view kProgram = R"( +fn main(x: u32) -> u32 { + match x { + () => u32:0, + (a) => a, + (b,) => b, + (c, (d, e)) => c, + } +})"; + + FileTable file_table; + XLS_ASSERT_OK_AND_ASSIGN(auto module, ParseModule(kProgram, "fake_path.x", + "the_module", file_table)); + XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr clone, + CloneModule(*module.get())); + XLS_ASSERT_OK_AND_ASSIGN(Function * main, + clone->GetMemberOrError("main")); + Statement* statement = main->body()->statements().back(); + auto* match = absl::down_cast(std::get(statement->wrapped())); + ASSERT_EQ(match->arms().size(), 4); + + const PatternTree& empty_pattern = match->arms()[0]->patterns()[0]; + ASSERT_TRUE(std::holds_alternative(empty_pattern)); + EXPECT_TRUE(std::get(empty_pattern)->members().empty()); + + const PatternTree& singleton_pattern = match->arms()[1]->patterns()[0]; + ASSERT_TRUE(std::holds_alternative(singleton_pattern)); + EXPECT_EQ(std::get(singleton_pattern)->members().size(), 1); + + const PatternTree& trailing_pattern = match->arms()[2]->patterns()[0]; + ASSERT_TRUE(std::holds_alternative(trailing_pattern)); + EXPECT_EQ(std::get(trailing_pattern)->members().size(), 1); + + const PatternTree& nested_pattern = match->arms()[3]->patterns()[0]; + ASSERT_TRUE(std::holds_alternative(nested_pattern)); + TuplePattern* outer = std::get(nested_pattern); + ASSERT_EQ(outer->members().size(), 2); + ASSERT_TRUE(std::holds_alternative(outer->members()[1])); + TuplePattern* inner = std::get(outer->members()[1]); + ASSERT_EQ(inner->members().size(), 2); + EXPECT_EQ(inner->parent(), outer); + + for (MatchArm* arm : match->arms()) { + for (NameDef* name_def : GetPatternNameDefs(arm->patterns()[0])) { + EXPECT_EQ(name_def->definer(), arm); + } + } +} + TEST(AstClonerTest, LetMatch) { constexpr std::string_view kProgram = R"(import foo; fn main(x: u32, y: u32) -> u32 { diff --git a/xls/dslx/frontend/ast_node.h b/xls/dslx/frontend/ast_node.h index d1885f40f7..56cba3cc6b 100644 --- a/xls/dslx/frontend/ast_node.h +++ b/xls/dslx/frontend/ast_node.h @@ -65,7 +65,6 @@ enum class AstNodeKind : uint8_t { kMatchArm, kModule, kNameDef, - kNameDefTree, kNameRef, kNumber, kParam, @@ -95,6 +94,7 @@ enum class AstNodeKind : uint8_t { kTestFunction, kTestProc, kTrait, + kTuplePattern, kTupleIndex, kTypeAlias, kTypeAnnotation, diff --git a/xls/dslx/frontend/ast_test.cc b/xls/dslx/frontend/ast_test.cc index 741551829a..e08b45d5bf 100644 --- a/xls/dslx/frontend/ast_test.cc +++ b/xls/dslx/frontend/ast_test.cc @@ -20,10 +20,10 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/status/status.h" #include "absl/status/status_matchers.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" #include "xls/common/attribute_data.h" #include "xls/common/status/matchers.h" #include "xls/dslx/frontend/ast_test_utils.h" @@ -235,6 +235,30 @@ TEST_F(AstTest, GetFuncParam) { HasSubstr("Param 'not_a_param' not a parameter"))); } +TEST_F(AstTest, ConstPatternTreeFlattensToConstLeaves) { + NameDef* first = + m.Make(fake_span, std::string("first"), /*definer=*/nullptr); + WildcardPattern* wildcard = m.Make(fake_span); + NameDef* second = + m.Make(fake_span, std::string("second"), /*definer=*/nullptr); + TuplePattern* nested = m.Make( + fake_span, std::vector{wildcard, second}); + const TuplePattern* root = + m.Make(fake_span, std::vector{first, nested}); + + ConstPatternTree pattern = root; + std::vector leaves = FlattenPattern(pattern); + ASSERT_EQ(leaves.size(), 3); + EXPECT_EQ(std::get(leaves[0]), first); + EXPECT_EQ(std::get(leaves[1]), wildcard); + EXPECT_EQ(std::get(leaves[2]), second); + + std::vector name_defs = GetPatternNameDefs(pattern); + ASSERT_EQ(name_defs.size(), 2); + EXPECT_EQ(name_defs[0], first); + EXPECT_EQ(name_defs[1], second); +} + TEST_F(AstTest, IsConstantNameRef) { NameDef* name_def = m.Make(fake_span, std::string("MyStruct"), nullptr); diff --git a/xls/dslx/frontend/ast_utils_test.cc b/xls/dslx/frontend/ast_utils_test.cc index 81008404bf..15dfaadac5 100644 --- a/xls/dslx/frontend/ast_utils_test.cc +++ b/xls/dslx/frontend/ast_utils_test.cc @@ -21,9 +21,9 @@ #include #include -#include "gtest/gtest.h" #include "absl/base/casts.h" #include "absl/log/log.h" +#include "gtest/gtest.h" #include "xls/common/status/matchers.h" #include "xls/dslx/frontend/ast.h" #include "xls/dslx/frontend/module.h" @@ -173,7 +173,7 @@ fn f() -> u32 { auto* match = absl::down_cast(match_expr); XLS_ASSERT_OK_AND_ASSIGN(std::vector nodes, CollectUnder(match, /*want_types=*/false)); - ASSERT_EQ(nodes.size(), 15); + ASSERT_EQ(nodes.size(), 12); EXPECT_EQ(nodes[0]->ToString(), "t"); EXPECT_EQ(nodes[0]->GetNodeTypeName(), "NameRef"); @@ -181,45 +181,36 @@ fn f() -> u32 { EXPECT_EQ(nodes[1]->ToString(), "x"); EXPECT_EQ(nodes[1]->GetNodeTypeName(), "NameDef"); - EXPECT_EQ(nodes[2]->ToString(), "x"); - EXPECT_EQ(nodes[2]->GetNodeTypeName(), "NameDefTree"); + EXPECT_EQ(nodes[2]->ToString(), "u32"); + EXPECT_EQ(nodes[2]->GetNodeTypeName(), "BuiltinTypeAnnotation"); - EXPECT_EQ(nodes[3]->ToString(), "u32"); - EXPECT_EQ(nodes[3]->GetNodeTypeName(), "BuiltinTypeAnnotation"); + EXPECT_EQ(nodes[3]->ToString(), "u32:0"); + EXPECT_EQ(nodes[3]->GetNodeTypeName(), "Number"); - EXPECT_EQ(nodes[4]->ToString(), "u32:0"); - EXPECT_EQ(nodes[4]->GetNodeTypeName(), "Number"); + EXPECT_EQ(nodes[4]->ToString(), "x => u32:0"); + EXPECT_EQ(nodes[4]->GetNodeTypeName(), "MatchArm"); - EXPECT_EQ(nodes[5]->ToString(), "x => u32:0"); - EXPECT_EQ(nodes[5]->GetNodeTypeName(), "MatchArm"); + EXPECT_EQ(nodes[5]->ToString(), "y"); + EXPECT_EQ(nodes[5]->GetNodeTypeName(), "NameDef"); - EXPECT_EQ(nodes[6]->ToString(), "y"); + EXPECT_EQ(nodes[6]->ToString(), "z"); EXPECT_EQ(nodes[6]->GetNodeTypeName(), "NameDef"); - EXPECT_EQ(nodes[7]->ToString(), "y"); - EXPECT_EQ(nodes[7]->GetNodeTypeName(), "NameDefTree"); - - EXPECT_EQ(nodes[8]->ToString(), "z"); - EXPECT_EQ(nodes[8]->GetNodeTypeName(), "NameDef"); - - EXPECT_EQ(nodes[9]->ToString(), "z"); - EXPECT_EQ(nodes[9]->GetNodeTypeName(), "NameDefTree"); - - EXPECT_EQ(nodes[10]->ToString(), "(y, z)"); - EXPECT_EQ(nodes[10]->GetNodeTypeName(), "NameDefTree"); + EXPECT_EQ(nodes[7]->ToString(), "(y, z)"); + EXPECT_EQ(nodes[7]->GetNodeTypeName(), "TuplePattern"); - EXPECT_EQ(nodes[11]->ToString(), "u32"); - EXPECT_EQ(nodes[11]->GetNodeTypeName(), "BuiltinTypeAnnotation"); + EXPECT_EQ(nodes[8]->ToString(), "u32"); + EXPECT_EQ(nodes[8]->GetNodeTypeName(), "BuiltinTypeAnnotation"); - EXPECT_EQ(nodes[12]->ToString(), "u32:1"); - EXPECT_EQ(nodes[12]->GetNodeTypeName(), "Number"); + EXPECT_EQ(nodes[9]->ToString(), "u32:1"); + EXPECT_EQ(nodes[9]->GetNodeTypeName(), "Number"); - EXPECT_EQ(nodes[13]->ToString(), "(y, z) => u32:1"); - EXPECT_EQ(nodes[13]->GetNodeTypeName(), "MatchArm"); + EXPECT_EQ(nodes[10]->ToString(), "(y, z) => u32:1"); + EXPECT_EQ(nodes[10]->GetNodeTypeName(), "MatchArm"); - EXPECT_EQ(nodes[14]->ToString(), + EXPECT_EQ(nodes[11]->ToString(), "match t {\n x => u32:0,\n (y, z) => u32:1,\n}"); - EXPECT_EQ(nodes[14]->GetNodeTypeName(), "Match"); + EXPECT_EQ(nodes[11]->GetNodeTypeName(), "Match"); } // Tests that the ResolveLocalStructDef can see through transitive aliases. diff --git a/xls/dslx/frontend/function_specializer.cc b/xls/dslx/frontend/function_specializer.cc index a148f46547..216446d089 100644 --- a/xls/dslx/frontend/function_specializer.cc +++ b/xls/dslx/frontend/function_specializer.cc @@ -157,8 +157,8 @@ class SyntheticSpanAllocator { const_cast(name_def->span()) = span; return absl::OkStatus(); } - if (auto* name_def_tree = dynamic_cast(node)) { - const_cast(name_def_tree->span()) = span; + if (auto* tuple_pattern = dynamic_cast(node)) { + const_cast(tuple_pattern->span()) = span; return absl::OkStatus(); } if (auto* wildcard = dynamic_cast(node)) { diff --git a/xls/dslx/frontend/parser.cc b/xls/dslx/frontend/parser.cc index e5101b3e53..3ebbce5275 100644 --- a/xls/dslx/frontend/parser.cc +++ b/xls/dslx/frontend/parser.cc @@ -1933,31 +1933,30 @@ absl::StatusOr Parser::ParseNameDef(Bindings& bindings) { return name_def; } -absl::StatusOr Parser::ParseNameDefTree(Bindings& bindings) { +absl::StatusOr Parser::ParseNameDefPattern(Bindings& bindings) { XLS_ASSIGN_OR_RETURN(Token start, PopTokenOrError(TokenKind::kOParen)); - auto parse_name_def_or_tree = [&bindings, - this]() -> absl::StatusOr { + auto parse_binding_pattern_member = [&bindings, + this]() -> absl::StatusOr { XLS_ASSIGN_OR_RETURN(const Token* peek, PeekToken()); if (peek->kind() == TokenKind::kOParen) { XLS_ASSIGN_OR_RETURN(ExpressionDepthGuard expr_depth, BumpExpressionDepth()); - return ParseNameDefTree(bindings); + return ParseNameDefPattern(bindings); } XLS_ASSIGN_OR_RETURN(auto name_def, ParseNameDefOrWildcard(bindings)); - auto tree_leaf = WidenVariantTo(name_def); - return module_->Make(GetSpan(name_def), tree_leaf); + return WidenVariantTo(name_def); }; - XLS_ASSIGN_OR_RETURN( - std::vector branches, - ParseCommaSeq(parse_name_def_or_tree, TokenKind::kCParen)); - NameDefTree* ndt = module_->Make( + XLS_ASSIGN_OR_RETURN(std::vector branches, + ParseCommaSeq(parse_binding_pattern_member, + TokenKind::kCParen)); + PatternTree pattern = module_->Make( Span(start.span().start(), GetPos()), std::move(branches)); // Check that the name definitions are unique -- can't bind the same name // multiple times in one destructuring assignment. - std::vector name_defs = ndt->GetNameDefs(); + std::vector name_defs = GetPatternNameDefs(pattern); absl::flat_hash_map seen; for (NameDef* name_def : name_defs) { if (!seen.insert({name_def->identifier(), name_def}).second) { @@ -1969,7 +1968,7 @@ absl::StatusOr Parser::ParseNameDefTree(Bindings& bindings) { seen[name_def->identifier()]->span().ToString(file_table()))); } } - return ndt; + return pattern; } absl::StatusOr Parser::ParseArray(Bindings& bindings) { @@ -2234,22 +2233,23 @@ absl::StatusOr Parser::ParseComparisonExpression( return lhs; } -absl::StatusOr Parser::ParsePattern(Bindings& bindings, - bool within_tuple_pattern) { +absl::StatusOr Parser::ParsePattern(Bindings& bindings, + bool within_tuple_pattern) { XLS_ASSIGN_OR_RETURN(ExpressionDepthGuard depth_guard, BumpExpressionDepth()); XLS_ASSIGN_OR_RETURN(std::optional oparen, TryPopToken(TokenKind::kOParen)); if (oparen.has_value()) { - return ParseTuplePattern(oparen->span().start(), bindings); + XLS_ASSIGN_OR_RETURN(TuplePattern * tuple, + ParseTuplePattern(oparen->span().start(), bindings)); + return tuple; } XLS_ASSIGN_OR_RETURN(const Token* peek, PeekToken()); if (peek->kind() == TokenKind::kIdentifier) { XLS_ASSIGN_OR_RETURN(Token tok, PopTokenOrError(TokenKind::kIdentifier)); if (*tok.GetValue() == "_") { - return module_->Make( - tok.span(), module_->Make(tok.span())); + return module_->Make(tok.span()); } // TODO: https://github.com/google/xls/issues/1459 - Handle rest-of-tuple. XLS_ASSIGN_OR_RETURN(bool peek_is_double_colon, @@ -2258,8 +2258,7 @@ absl::StatusOr Parser::ParsePattern(Bindings& bindings, XLS_ASSIGN_OR_RETURN(NameRef * subject, ParseNameRef(bindings, &tok)); XLS_ASSIGN_OR_RETURN(ColonRef * colon_ref, ParseColonRef(bindings, subject, subject->span())); - Span span(tok.span().start(), colon_ref->span().limit()); - return module_->Make(span, colon_ref); + return colon_ref; } std::string identifier = tok.GetValue().value(); @@ -2269,16 +2268,13 @@ absl::StatusOr Parser::ParsePattern(Bindings& bindings, bindings.ResolveNameOrNullopt(identifier).value(); NameRef* ref = module_->Make(tok.span(), identifier, any_name_def); - return module_->Make(tok.span(), ref); + return ref; } // If the name is not bound, this pattern is creating a binding. XLS_ASSIGN_OR_RETURN(NameDef * name_def, TokenToNameDef(tok)); bindings.Add(name_def->identifier(), name_def); - Span span(tok.span().start(), GetPos()); - auto* result = module_->Make(span, name_def); - name_def->set_definer(result); - return result; + return name_def; } if (peek->kind() == TokenKind::kDoubleDot) { @@ -2288,8 +2284,7 @@ absl::StatusOr Parser::ParsePattern(Bindings& bindings, "`..` patterns are not allowed outside of a tuple pattern"); } XLS_ASSIGN_OR_RETURN(Token rest, PopTokenOrError(TokenKind::kDoubleDot)); - return module_->Make(rest.span(), - module_->Make(rest.span())); + return module_->Make(rest.span()); } if (peek->IsKindIn({TokenKind::kNumber, TokenKind::kCharacter, Keyword::kTrue, @@ -2307,9 +2302,9 @@ absl::StatusOr Parser::ParsePattern(Bindings& bindings, Span(number->span().start(), limit->span().limit()), number, peek_is_double_dot_equals, limit, /*in_parens=*/false, /*pattern_semantics=*/true); - return module_->Make(range->span(), range); + return range; } - return module_->Make(number->span(), number); + return number; } return ParseErrorStatus( @@ -2341,9 +2336,11 @@ absl::StatusOr Parser::ParseMatch(Bindings& bindings, bool is_const) { } Bindings arm_bindings(&bindings); XLS_ASSIGN_OR_RETURN( - NameDefTree * first_pattern, + PatternTree first_pattern, ParsePattern(arm_bindings, /*within_tuple_pattern=*/false)); - std::vector patterns = {first_pattern}; + const Span first_pattern_span(GetPatternSpan(first_pattern).start(), + GetPos()); + std::vector patterns = {first_pattern}; while (true) { XLS_ASSIGN_OR_RETURN(bool dropped_bar, TryDropToken(TokenKind::kBar)); if (!dropped_bar) { @@ -2355,13 +2352,13 @@ absl::StatusOr Parser::ParseMatch(Bindings& bindings, bool is_const) { std::vector locals = MapKeysSorted(arm_bindings.local_bindings()); return ParseErrorStatus( - first_pattern->span(), + first_pattern_span, absl::StrFormat("Cannot have multiple patterns that bind names; " "previously bound: %s", absl::StrJoin(locals, ", "))); } XLS_ASSIGN_OR_RETURN( - NameDefTree * pattern, + PatternTree pattern, ParsePattern(arm_bindings, /*within_tuple_pattern=*/false)); patterns.push_back(pattern); } @@ -2370,9 +2367,15 @@ absl::StatusOr Parser::ParseMatch(Bindings& bindings, bool is_const) { // The span of the match arm is from the start of the pattern to the end of // the RHS expression. - Span span(patterns[0]->span().start(), rhs->span().limit()); + Span span(GetPatternSpan(patterns[0]).start(), rhs->span().limit()); - arms.push_back(module_->Make(span, std::move(patterns), rhs)); + MatchArm* arm = module_->Make(span, std::move(patterns), rhs); + for (const PatternTree& pattern : arm->patterns()) { + for (NameDef* name_def : GetPatternNameDefs(pattern)) { + name_def->set_definer(arm); + } + } + arms.push_back(arm); XLS_ASSIGN_OR_RETURN(bool dropped_comma, TryDropToken(TokenKind::kComma)); must_end = !dropped_comma; } @@ -4193,30 +4196,27 @@ absl::StatusOr Parser::ParseLet(Bindings& bindings) { Bindings new_bindings(&bindings); NameDef* name_def = nullptr; - NameDefTree* name_def_tree; + std::optional pattern; XLS_ASSIGN_OR_RETURN(bool peek_is_oparen, PeekTokenIs(TokenKind::kOParen)); if (peek_is_oparen) { // Destructuring binding. - XLS_ASSIGN_OR_RETURN(name_def_tree, ParseNameDefTree(new_bindings)); + XLS_ASSIGN_OR_RETURN(pattern, ParseNameDefPattern(new_bindings)); } else { XLS_ASSIGN_OR_RETURN(name_def, ParseNameDef(new_bindings)); if (name_def->identifier() == "_") { - name_def_tree = module_->Make( - name_def->span(), module_->Make(name_def->span())); + pattern = module_->Make(name_def->span()); } else { - name_def_tree = module_->Make(name_def->span(), name_def); + pattern = name_def; } } if (const_) { - // Mark this NDT as const. Also disallow destructuring assignment for - // constants. - const_ndts_.insert(name_def_tree); - if (name_def_tree->Flatten().size() != 1) { + // Constant definitions cannot use destructuring assignment. + if (FlattenPattern(*pattern).size() != 1) { return ParseErrorStatus( - name_def_tree->span(), + GetPatternSpan(*pattern), absl::StrFormat( "Constant definitions can not use destructuring assignment: %s", - name_def_tree->ToString())); + PatternToString(*pattern))); } } @@ -4231,8 +4231,7 @@ absl::StatusOr Parser::ParseLet(Bindings& bindings) { XLS_RETURN_IF_ERROR(DropTokenOrError(TokenKind::kSemi)); Span span(start_tok.span().start(), GetPos()); - Let* let = - module_->Make(span, name_def_tree, annotated_type, rhs, const_); + Let* let = module_->Make(span, *pattern, annotated_type, rhs, const_); if (const_ && name_def != nullptr) { name_def->set_definer(let); } else if (name_def != nullptr) { @@ -4255,7 +4254,7 @@ absl::StatusOr Parser::ParseFor(Bindings& bindings) { XLS_RET_CHECK(for_kw.IsKeyword(Keyword::kFor) || is_unroll_for); Bindings for_bindings(&bindings); - XLS_ASSIGN_OR_RETURN(NameDefTree * names, ParseNameDefTree(for_bindings)); + XLS_ASSIGN_OR_RETURN(PatternTree pattern, ParseNameDefPattern(for_bindings)); XLS_ASSIGN_OR_RETURN(bool peek_is_colon, PeekTokenIs(TokenKind::kColon)); TypeAnnotation* type = nullptr; if (peek_is_colon) { @@ -4282,10 +4281,11 @@ absl::StatusOr Parser::ParseFor(Bindings& bindings) { XLS_RETURN_IF_ERROR(DropTokenOrError(TokenKind::kCParen)); if (is_const_for || is_unroll_for) { - return module_->Make(Span(for_kw.span().start(), GetPos()), names, - type, iterable, body, init, is_unroll_for); + return module_->Make(Span(for_kw.span().start(), GetPos()), + pattern, type, iterable, body, init, + is_unroll_for); } else { - return module_->Make(Span(for_kw.span().start(), GetPos()), names, + return module_->Make(Span(for_kw.span().start(), GetPos()), pattern, type, iterable, body, init); } } @@ -4652,9 +4652,9 @@ absl::StatusOr Parser::ParseTrait(const Pos& start_pos, bool is_public, return module_->Make(span, name_def, std::move(members), is_public); } -absl::StatusOr Parser::ParseTuplePattern(const Pos& start_pos, - Bindings& bindings) { - std::vector members; +absl::StatusOr Parser::ParseTuplePattern(const Pos& start_pos, + Bindings& bindings) { + std::vector members; bool must_end = false; bool rest_of_tuple_seen = false; while (true) { @@ -4666,12 +4666,12 @@ absl::StatusOr Parser::ParseTuplePattern(const Pos& start_pos, XLS_RETURN_IF_ERROR(DropTokenOrError(TokenKind::kCParen)); break; } - XLS_ASSIGN_OR_RETURN(NameDefTree * pattern, + XLS_ASSIGN_OR_RETURN(PatternTree pattern, ParsePattern(bindings, /*within_tuple_pattern=*/true)); - if (pattern->IsRestOfTupleLeaf()) { + if (IsRestOfTupleLeaf(pattern)) { if (rest_of_tuple_seen) { return ParseErrorStatus( - pattern->span(), + GetPatternSpan(pattern), "Rest-of-tuple (`..`) can only be used once per tuple pattern."); } rest_of_tuple_seen = true; @@ -4681,7 +4681,7 @@ absl::StatusOr Parser::ParseTuplePattern(const Pos& start_pos, must_end = !dropped_comma; } Span span(start_pos, GetPos()); - return module_->Make(span, std::move(members)); + return module_->Make(span, std::move(members)); } absl::StatusOr Parser::ParseBlockExpression(Bindings& bindings, @@ -4922,15 +4922,4 @@ absl::StatusOr> Parser::ParseParametrics( TokenKind::kCAngle); } -const Span& GetSpan( - const std::variant& v) { - if (std::holds_alternative(v)) { - return std::get(v)->span(); - } - if (std::holds_alternative(v)) { - return std::get(v)->span(); - } - return std::get(v)->span(); -} - } // namespace xls::dslx diff --git a/xls/dslx/frontend/parser.h b/xls/dslx/frontend/parser.h index 4c3faf69e9..dded8c55a2 100644 --- a/xls/dslx/frontend/parser.h +++ b/xls/dslx/frontend/parser.h @@ -349,14 +349,14 @@ class Parser : public TokenParser { absl::StatusOr> ParseNameDefOrWildcard(Bindings& bindings); - // Parses tree of name defs and returns it. + // Parses a binding pattern and returns its PatternTree representation. // // For example, the left hand side of: // // let (a, (b, (c)), d) = ... // - // This is used for tuple-like (sometimes known as "destructing") let binding. - absl::StatusOr ParseNameDefTree(Bindings& bindings); + // This is used for tuple-like destructuring let binding. + absl::StatusOr ParseNameDefPattern(Bindings& bindings); absl::StatusOr TokenToNumber(const Token& tok); absl::StatusOr TokenToNameDef(const Token& tok) { @@ -543,8 +543,8 @@ class Parser : public TokenParser { // Permits trailing commas. absl::StatusOr> ParseParams(Bindings& bindings); - absl::StatusOr ParseTuplePattern(const Pos& start_pos, - Bindings& bindings); + absl::StatusOr ParseTuplePattern(const Pos& start_pos, + Bindings& bindings); // Returns a parsed pattern; e.g. one that would guard a match arm. // @@ -554,8 +554,8 @@ class Parser : public TokenParser { // | NameDef // | NameRef // | Number - absl::StatusOr ParsePattern(Bindings& bindings, - bool within_tuple_pattern); + absl::StatusOr ParsePattern(Bindings& bindings, + bool within_tuple_pattern); // Parses a match expression. absl::StatusOr ParseMatch(Bindings& bindings, bool is_const); @@ -766,13 +766,6 @@ class Parser : public TokenParser { // (sub-parser), it points to the borrowed module. Module* const module_; - // `Let` nodes are created _after_ those that use their namedefs (due to the - // chaining of the `body` member variable. We need to know, though, if a - // reference to such an NDT (or element thereof) is to a constant or not, so - // we can emit a NameRef. This set holds those NDTs known - // to be constant for that purpose. - absl::flat_hash_set const_ndts_; - // To avoid over-recursion (and ensuing stack overflows) we keep track of the // approximate expression depth, and bail when expressions are unreasonably // deeply nested. @@ -784,9 +777,6 @@ class Parser : public TokenParser { bool parse_fn_stubs_; }; -const Span& GetSpan( - const std::variant& v); - } // namespace xls::dslx #endif // XLS_DSLX_FRONTEND_PARSER_H_ diff --git a/xls/dslx/frontend/parser_test.cc b/xls/dslx/frontend/parser_test.cc index 2fa03ab6ad..478469cc05 100644 --- a/xls/dslx/frontend/parser_test.cc +++ b/xls/dslx/frontend/parser_test.cc @@ -22,9 +22,6 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest-spi.h" -#include "gtest/gtest.h" #include "absl/base/casts.h" #include "absl/container/flat_hash_set.h" #include "absl/log/log.h" @@ -32,6 +29,9 @@ #include "absl/status/status_matchers.h" #include "absl/status/statusor.h" #include "absl/types/span.h" +#include "gmock/gmock.h" +#include "gtest/gtest-spi.h" +#include "gtest/gtest.h" #include "xls/common/attribute_data.h" #include "xls/common/file/filesystem.h" #include "xls/common/file/get_runfile_path.h" @@ -1021,7 +1021,7 @@ TEST_F(ParserTest, ParseLet) { ASSERT_EQ(stmts.size(), 2); Let* let = std::get(stmts.at(0)->wrapped()); - NameDef* name_def = std::get(let->name_def_tree()->leaf()); + NameDef* name_def = std::get(let->pattern()); EXPECT_EQ(name_def->identifier(), "x"); EXPECT_EQ(let->type_annotation()->ToString(), "u32"); EXPECT_EQ(let->rhs()->ToString(), "2"); @@ -1046,13 +1046,83 @@ TEST_F(ParserTest, ParseLetWildcardBinding) { ASSERT_EQ(stmts.size(), 1); Let* let = std::get(stmts.at(0)->wrapped()); - EXPECT_EQ( - AstNodeKindToString(ToAstNode(let->name_def_tree()->leaf())->kind()), - "wildcard pattern"); - WildcardPattern* wildcard = - std::get(let->name_def_tree()->leaf()); + EXPECT_EQ(AstNodeKindToString(ToAstNode(let->pattern())->kind()), + "wildcard pattern"); + WildcardPattern* wildcard = std::get(let->pattern()); ASSERT_NE(wildcard, nullptr); - ASSERT_TRUE(let->name_def_tree()->IsWildcardLeaf()); + ASSERT_TRUE(IsWildcardLeaf(let->pattern())); +} + +TEST_F(ParserTest, PatternTreePreservesParenthesizedShape) { + const char* text = R"({ + let direct = x; + let () = x; + let (singleton) = x; + let (trailing,) = x; + let (a, (b, c)) = x; + direct +})"; + Scanner s{file_table_, Fileno(0), std::string{text}}; + Parser p{"test", &s}; + Bindings b; + b.Add("x", p.module().GetOrCreateBuiltinNameDef("x")); + XLS_ASSERT_OK_AND_ASSIGN(StatementBlock * block, + p.ParseBlockExpression(/*bindings=*/b)); + + absl::Span stmts = block->statements(); + ASSERT_EQ(stmts.size(), 6); + + Let* direct = std::get(stmts.at(0)->wrapped()); + ASSERT_TRUE(std::holds_alternative(direct->pattern())); + EXPECT_EQ(ToAstNode(direct->pattern())->parent(), direct); + + Let* empty = std::get(stmts.at(1)->wrapped()); + ASSERT_TRUE(std::holds_alternative(empty->pattern())); + EXPECT_TRUE(std::get(empty->pattern())->members().empty()); + + Let* singleton = std::get(stmts.at(2)->wrapped()); + ASSERT_TRUE(std::holds_alternative(singleton->pattern())); + TuplePattern* singleton_pattern = + std::get(singleton->pattern()); + ASSERT_EQ(singleton_pattern->members().size(), 1); + EXPECT_TRUE( + std::holds_alternative(singleton_pattern->members()[0])); + + Let* trailing = std::get(stmts.at(3)->wrapped()); + ASSERT_TRUE(std::holds_alternative(trailing->pattern())); + TuplePattern* trailing_pattern = std::get(trailing->pattern()); + ASSERT_EQ(trailing_pattern->members().size(), 1); + EXPECT_TRUE(std::holds_alternative(trailing_pattern->members()[0])); + + Let* nested = std::get(stmts.at(4)->wrapped()); + ASSERT_TRUE(std::holds_alternative(nested->pattern())); + TuplePattern* outer = std::get(nested->pattern()); + ASSERT_EQ(outer->members().size(), 2); + ASSERT_TRUE(std::holds_alternative(outer->members()[1])); + TuplePattern* inner = std::get(outer->members()[1]); + ASSERT_EQ(inner->members().size(), 2); + EXPECT_EQ(ToAstNode(outer->members()[0])->parent(), outer); + EXPECT_EQ(ToAstNode(inner->members()[0])->parent(), inner); + EXPECT_EQ(outer->parent(), nested); +} + +TEST_F(ParserTest, MatchPatternNamesUseArmDefiner) { + XLS_ASSERT_OK_AND_ASSIGN( + Expr * e, ParseExpr("match x { (a, (b, c)) => a }", /*predefine=*/{"x"})); + auto* match = absl::down_cast(e); + MatchArm* arm = match->arms()[0]; + const PatternTree& pattern = arm->patterns()[0]; + ASSERT_TRUE(std::holds_alternative(pattern)); + TuplePattern* outer = std::get(pattern); + ASSERT_TRUE(std::holds_alternative(outer->members()[1])); + TuplePattern* inner = std::get(outer->members()[1]); + + EXPECT_EQ(outer->parent(), arm); + EXPECT_EQ(ToAstNode(outer->members()[0])->parent(), outer); + EXPECT_EQ(ToAstNode(inner->members()[0])->parent(), inner); + for (NameDef* name_def : GetPatternNameDefs(pattern)) { + EXPECT_EQ(name_def->definer(), arm); + } } TEST_F(ParserTest, ParseLetWildcardBindingTwice) { @@ -1092,7 +1162,7 @@ TEST_F(ParserTest, ParseLetExpressionWithShadowing) { auto* name_ref = dynamic_cast(e); EXPECT_EQ(name_ref->ToString(), "x"); EXPECT_EQ(std::get(name_ref->name_def()), - std::get(second_let->name_def_tree()->leaf())); + std::get(second_let->pattern())); } TEST_F(ParserTest, ParseBlockMultiLet) { @@ -3024,6 +3094,16 @@ TEST_F(ParserTest, ColonRef) { EXPECT_EQ(e->span(), Span(Pos(Fileno(0), 0, 0), Pos(Fileno(0), 0, 18))); } +TEST_F(ParserTest, ColonRefPatternStaysTokenDirected) { + XLS_ASSERT_OK_AND_ASSIGN( + Expr * e, ParseExpr("match x { BuiltinEnum::VALUE => u32:0, _ => u32:1 }", + /*predefine=*/{"x", "BuiltinEnum"})); + auto* match = absl::down_cast(e); + const PatternTree& pattern = match->arms()[0]->patterns()[0]; + ASSERT_TRUE(std::holds_alternative(pattern)); + EXPECT_EQ(std::get(pattern)->ToString(), "BuiltinEnum::VALUE"); +} + TEST_F(ParserTest, ParametricColonRefInvocation) { RoundTripExpr("f()", {"f", "BuiltinEnum"}); } @@ -3498,7 +3578,7 @@ TEST_F(ParserTest, CastVsUnaryPrecedence) { EXPECT_EQ(cast->type_annotation()->ToString(), "s32"); } -TEST_F(ParserTest, NameDefTree) { +TEST_F(ParserTest, PatternTree) { RoundTripExpr(R"({ let (a, (b, (c, d), e), f) = x; a @@ -4483,7 +4563,7 @@ TEST_F(ParserTest, ParseWithUncompletedStateTransaction) { } // This sample would previously cause a segfault because we'd try to set a -// definer on a NameDef binding but we only have a NameDefTree. +// definer on a NameDef binding but we only have a tuple pattern. TEST_F(ParserTest, LocalConstWithNoNameDef) { constexpr std::string_view kProgram = "fn f(){const(X)=();}"; Scanner s{file_table_, Fileno(0), std::string(kProgram)}; diff --git a/xls/dslx/frontend/semantics_analysis.cc b/xls/dslx/frontend/semantics_analysis.cc index edbb1f6c45..810127dafd 100644 --- a/xls/dslx/frontend/semantics_analysis.cc +++ b/xls/dslx/frontend/semantics_analysis.cc @@ -479,8 +479,8 @@ class PreTypecheckPass : public AstNodeVisitorWithDefault { } absl::Status HandleLet(const Let* node) override { - if (node->name_def_tree()->IsWildcardLeaf()) { - warning_collector_.Add(node->name_def_tree()->span(), + if (IsWildcardLeaf(node->pattern())) { + warning_collector_.Add(GetPatternSpan(node->pattern()), WarningKind::kUselessLetBinding, "`let _ = expr;` statement can be simplified to " "`expr;` -- there is no " @@ -488,7 +488,7 @@ class PreTypecheckPass : public AstNodeVisitorWithDefault { } if (node->is_const()) { - NameDef* name_def = node->name_def_tree()->GetNameDefs()[0]; + NameDef* name_def = GetPatternNameDefs(node->pattern())[0]; WarnOnInappropriateConstantName(name_def->identifier(), node->span(), *node->owner(), &warning_collector_); } @@ -555,23 +555,18 @@ class CollectUseDef : public AstNodeVisitorWithDefault { return; } - // We make an exception to NameDefTree, that if any def in it is used, all - // defs in the tree are considered used. This is because the user - // typically wants to keep a meaningful name for each component of a - // NameDefTree binding, even though only some of them will be used, and we - // want to reduce superfluous warnings on that. + // If any name in a tuple binding is used, consider all names in that + // binding used. Users typically want to keep meaningful names for each + // component even when only some components are referenced. const AstNode* node = *name_def; while (node->parent() && - node->parent()->kind() == AstNodeKind::kNameDefTree) { - node = absl::down_cast(node->parent()); + node->parent()->kind() == AstNodeKind::kTuplePattern) { + node = absl::down_cast(node->parent()); } if (node != *name_def) { - for (NameDefTree::Leaf& leaf : - absl::down_cast(node)->Flatten()) { - if (const NameDef* const* tree_name_def = - std::get_if(&leaf)) { - uses_.insert(*tree_name_def); - } + ConstPatternTree pattern = absl::down_cast(node); + for (const NameDef* pattern_name_def : GetPatternNameDefs(pattern)) { + uses_.insert(pattern_name_def); } } } diff --git a/xls/dslx/ir_convert/function_converter.cc b/xls/dslx/ir_convert/function_converter.cc index 842fd9a0aa..09f6039352 100644 --- a/xls/dslx/ir_convert/function_converter.cc +++ b/xls/dslx/ir_convert/function_converter.cc @@ -115,15 +115,6 @@ absl::Status IrConversionErrorStatus(const std::optional& span, span ? span->ToString(file_table) : "", message)); } -// Convert a NameDefTree node variant to an AstNode pointer (either the leaf -// node or the interior NameDefTree node). -AstNode* ToAstNode(const std::variant& x) { - if (std::holds_alternative(x)) { - return std::get(x); - } - return ToAstNode(std::get(x)); -} - absl::StatusOr ToTypeDefinition( const TypeAnnotation* type_annotation) { auto* type_ref_type_annotation = @@ -420,7 +411,7 @@ class FunctionConverterVisitor : public AstNodeVisitor { INVALID(FuzzTestFunction) INVALID(MatchArm) INVALID(NameDef) - INVALID(NameDefTree) + INVALID(TuplePattern) INVALID(ParametricBinding) INVALID(ProcAlias) INVALID(RestOfTuple) @@ -970,9 +961,9 @@ absl::Status FunctionConverter::HandleLetChannelDecl(const Let* node) { "lowering to proc-scoped channels"; XLS_RETURN_IF_ERROR(ValidateProcState("chan", node)); - XLS_RET_CHECK(!node->name_def_tree()->is_leaf()) + XLS_RET_CHECK(std::holds_alternative(node->pattern())) << "Must assign a channel declaration to a 2-tuple; was leaf"; - std::vector leaves = node->name_def_tree()->Flatten(); + std::vector leaves = FlattenPattern(node->pattern()); XLS_RET_CHECK_EQ(leaves.size(), 2) << "Must assign a channel declaration to a 2-tuple"; @@ -1017,7 +1008,7 @@ absl::Status FunctionConverter::HandleLet(const Let* node) { node->span(), "Can only assign from channels when in a Proc", file_table()); } - if (!node->name_def_tree()->is_leaf()) { + if (std::holds_alternative(node->pattern())) { return absl::UnimplementedError( "Destructuring let bindings are not yet supported in Proc " "config methods."); @@ -1052,11 +1043,11 @@ absl::Status FunctionConverter::HandleLet(const Let* node) { }}, value)); - if (node->name_def_tree()->is_leaf()) { + if (!std::holds_alternative(node->pattern())) { // Alias so that the RHS expression is now known as the name definition it // is bound to. XLS_RETURN_IF_ERROR( - DefAlias(node->rhs(), /*to=*/ToAstNode(node->name_def_tree()->leaf()))); + DefAlias(node->rhs(), /*to=*/ToAstNode(node->pattern()))); } else { if (current_fn_tag_ == FunctionTag::kProcConfig) { return absl::UnimplementedError( @@ -1068,29 +1059,30 @@ absl::Status FunctionConverter::HandleLet(const Let* node) { // actually recursive (instead of "effectively recursive" via the `levels` // and `delta_at_level` vectors). - // Walk the tree of names we're trying to bind, performing tuple_index - // operations on the RHS to get to the values we want to bind to those - // names. + // Walk the pattern members, performing tuple_index operations on the RHS + // to get to the corresponding values. std::vector levels = {rhs}; - // Invoked at each level of the NameDefTree: binds the name in the - // NameDefTree to the corresponding value (being pattern matched). + // Invoked at each level of the pattern: maps the current pattern member + // to the corresponding value. Only NameDef leaves bind names; wildcard and + // rest members are visited without binding. // // Args: - // name_def_tree: Current subtree of the NameDefTree. - // level: Level (depth) in the NameDefTree, root is 0. + // pattern: Current subtree of the pattern. + // level: Level (depth) in the pattern, root is 0. // index: Index of node in the current tree level (e.g. leftmost is 0). // We need to add the delta to every tuple index *at this level*. E.g.,: - // if have a NameDefTree like: (a,..,b,(c,..,d),e), then b, (c,..,d) and + // if have a pattern like: (a,..,b,(c,..,d),e), then b, (c,..,d) and // e's indexes need to be adjusted at level 1; d's needs to be adjusted at // level 2, etc. std::vector delta_at_level; - auto walk = [&](NameDefTree* name_def_tree, int64_t level, + auto walk = [&](const PatternTree& pattern, int64_t level, int64_t index) -> absl::Status { - XLS_RET_CHECK_NE(name_def_tree, nullptr); + AstNode* pattern_node = ToAstNode(pattern); + XLS_RET_CHECK_NE(pattern_node, nullptr); VLOG(6) << absl::StreamFormat("Walking level %d index %d: `%s`", level, - index, name_def_tree->ToString()); + index, PatternToString(pattern)); while (level > delta_at_level.size()) { // Make sure we have enough entries at this level @@ -1105,18 +1097,18 @@ absl::Status FunctionConverter::HandleLet(const Let* node) { xls::TupleType* tuple_type = tuple.GetType()->AsTupleOrDie(); - if (name_def_tree->IsRestOfTupleLeaf()) { + if (IsRestOfTupleLeaf(pattern)) { // If we're at a "rest of tuple" operator, we may need to "skip" // tuple elements at this level. We can tell how many tuple *entries* - // there are via tuple_type, and how many *bindings* there are at this - // level (i.e., siblings to the "rest of tuple") via the parent - // NameDefTree. - const NameDefTree* parent = - absl::down_cast(name_def_tree->parent()); - int64_t number_of_bindings = parent->nodes().size(); + // there are via tuple_type, and how many pattern members there are at + // this level (including the "rest of tuple") via the parent tuple + // pattern. + const TuplePattern* parent = + absl::down_cast(pattern_node->parent()); + int64_t pattern_member_count = parent->members().size(); int64_t number_of_tuple_elements = tuple_type->size(); - int64_t delta = number_of_tuple_elements - number_of_bindings; + int64_t delta = number_of_tuple_elements - pattern_member_count; delta_at_level[level - 1] = delta; // Don't need to bind anything to the .. @@ -1126,29 +1118,27 @@ absl::Status FunctionConverter::HandleLet(const Let* node) { CHECK_LT(index, tuple_type->size()) << "index: " << index << " type: " << tuple_type->ToString(); - levels.push_back(Def(name_def_tree, [this, name_def_tree, index, - tuple](SourceInfo loc) { - if (!loc.Empty()) { - loc = ToSourceInfo(name_def_tree->is_leaf() - ? ToAstNode(name_def_tree->leaf())->GetSpan() - : name_def_tree->GetSpan()); - } - - CHECK(tuple.GetType()->IsTuple()); - BValue tuple_index = function_builder_->TupleIndex(tuple, index, loc); - CHECK_OK(function_builder_->GetError()); + BValue bound_value = + Def(pattern_node, [this, pattern_node, index, tuple](SourceInfo loc) { + if (!loc.Empty()) { + loc = ToSourceInfo(pattern_node->GetSpan()); + } - return tuple_index; - })); + CHECK(tuple.GetType()->IsTuple()); + BValue tuple_index = + function_builder_->TupleIndex(tuple, index, loc); + CHECK_OK(function_builder_->GetError()); - if (name_def_tree->is_leaf()) { - XLS_RETURN_IF_ERROR( - DefAlias(name_def_tree, ToAstNode(name_def_tree->leaf()))); + return tuple_index; + }); + if (std::holds_alternative(pattern)) { + bound_value.SetName(std::get(pattern)->identifier()); } + levels.push_back(bound_value); return absl::OkStatus(); }; - XLS_RETURN_IF_ERROR(node->name_def_tree()->DoPreorder(walk)); + XLS_RETURN_IF_ERROR(DoPatternPreorder(node->pattern(), walk)); } return absl::OkStatus(); @@ -1354,7 +1344,7 @@ absl::Status FunctionConverter::HandleMatch(const Match* node) { // Visit all the MatchArm's patterns. std::vector this_arm_selectors; - for (NameDefTree* pattern : arm->patterns()) { + for (const PatternTree& pattern : arm->patterns()) { XLS_ASSIGN_OR_RETURN(BValue selector, HandleMatcher(pattern, matched, *matched_type)); XLS_RET_CHECK(selector.valid()); @@ -1550,7 +1540,7 @@ absl::StatusOr FunctionConverter::GetRangeData( absl::StatusOr FunctionConverter::HandleRangedForInductionVariable( const For* node, FunctionConverter& body_converter, - NameDefTree::Leaf ivar_node) { + const PatternTree& ivar_node) { return absl::visit( Visitor{ [&](NameDef* name_def) -> absl::StatusOr { @@ -1581,6 +1571,10 @@ absl::StatusOr FunctionConverter::HandleRangedForInductionVariable( return absl::InternalError( "Induction variable cannot be a \"rest of tuple\""); }, + [&](TuplePattern*) -> absl::StatusOr { + return absl::InternalError( + "Induction variable cannot be a tuple pattern"); + }, }, ivar_node); } @@ -1588,7 +1582,7 @@ absl::StatusOr FunctionConverter::HandleRangedForInductionVariable( absl::Status FunctionConverter::HandleForLoopCarry( const For* node, FunctionConverter& body_converter, const std::optional& range_data, BValue loop_index, - NameDefTree::Leaf ivar, AstNode* carry_node) { + const PatternTree& ivar, AstNode* carry_node) { // IR `counted_for` ops only support a trip count, not a set of iterables, so // we need to add an offset to that trip count/index to support nonzero loop // start indices; e.g. `for (i, accum) in 1..10` we actually iterate from 0 to @@ -1623,10 +1617,9 @@ absl::Status FunctionConverter::HandleForLoopCarry( body_converter.SetNodeToIr(carry_name_def, param); } } else { - // For tuple loop carries we have to destructure names on entry. - // Note this could be something like a NameDef or something like a - // WildcardPattern -- even if it's a wildcard pattern we throw away, we - // still want to make the loop with the same pattern. + // Preserve the full loop-carry pattern on entry. Tuple members may bind + // names or be non-binding patterns; the latter produce no binding, but the + // carry keeps the same pattern shape. AstNode* accum = carry_node; XLS_ASSIGN_OR_RETURN(std::unique_ptr carry_type, ResolveType(accum)); XLS_ASSIGN_OR_RETURN( @@ -1639,17 +1632,17 @@ absl::Status FunctionConverter::HandleForLoopCarry( carry = body_converter.AddParam("__loop_carry", carry_ir_type); } body_converter.SetNodeToIr(accum, carry); - // This will destructure the names for us in the body of the anonymous - // function. - if (auto* ndt = dynamic_cast(accum)) { + // Destructure the tuple-pattern members in the anonymous function body. + if (auto* tuple_pattern = dynamic_cast(accum)) { + PatternTree matcher = tuple_pattern; XLS_RETURN_IF_ERROR(body_converter - .HandleMatcher(/*matcher=*/ndt, + .HandleMatcher(/*matcher=*/matcher, /*matched_value=*/carry, /*matched_type=*/*carry_type) .status()); } else { XLS_RET_CHECK_NE(dynamic_cast(accum), nullptr) - << "Expect post-typechecking loop binding to be NameDefTree or " + << "Expect post-typechecking loop binding to be TuplePattern or " "WildcardPattern"; } } @@ -1806,8 +1799,7 @@ absl::Status FunctionConverter::HandleFor(const For* node) { } // Grab the two tuple of `(ivar, accum)`. - std::vector> flat = - node->names()->Flatten1(); + std::vector flat = FlattenPattern1(node->pattern()); if (flat.size() != 2) { return absl::UnimplementedError( "Expect for loop to have counter (induction variable) and carry data " @@ -1815,7 +1807,7 @@ absl::Status FunctionConverter::HandleFor(const For* node) { } // Add the induction value (the "ranged" counter). - NameDefTree::Leaf ivar_node = std::get(flat[0]); + const PatternTree& ivar_node = flat[0]; AstNode* carry_node = ToAstNode(flat[1]); VLOG(5) << "Converting for-loop @ " << node->span().ToString(file_table()); @@ -1901,29 +1893,29 @@ absl::Status FunctionConverter::HandleFor(const For* node) { } absl::StatusOr FunctionConverter::HandleMatcher( - NameDefTree* matcher, const BValue& matched_value, + const PatternTree& matcher, const BValue& matched_value, const Type& matched_type) { - if (matcher->is_leaf()) { - NameDefTree::Leaf leaf = matcher->leaf(); + AstNode* matcher_node = ToAstNode(matcher); + if (!std::holds_alternative(matcher)) { VLOG(5) << absl::StreamFormat("Matcher is leaf: %s (%s)", - ToAstNode(leaf)->ToString(), - ToAstNode(leaf)->GetNodeTypeName()); + matcher_node->ToString(), + matcher_node->GetNodeTypeName()); auto equality = [&]() -> absl::StatusOr { - XLS_RETURN_IF_ERROR(Visit(ToAstNode(leaf))); - XLS_ASSIGN_OR_RETURN(BValue to_match, Use(ToAstNode(leaf))); - return Def(matcher, [&](const SourceInfo& loc) { + XLS_RETURN_IF_ERROR(Visit(matcher_node)); + XLS_ASSIGN_OR_RETURN(BValue to_match, Use(matcher_node)); + return Def(matcher_node, [&](const SourceInfo& loc) { return function_builder_->Eq(to_match, matched_value); }); }; return absl::visit( Visitor{ [&](WildcardPattern*) -> absl::StatusOr { - return Def(matcher, [&](const SourceInfo& loc) { + return Def(matcher_node, [&](const SourceInfo& loc) { return function_builder_->Literal(UBits(1, 1), loc); }); }, [&](RestOfTuple* n) -> absl::StatusOr { - return Def(matcher, [&](const SourceInfo& loc) { + return Def(matcher_node, [&](const SourceInfo& loc) { return function_builder_->Literal(UBits(1, 1), loc); }); }, @@ -1971,40 +1963,44 @@ absl::StatusOr FunctionConverter::HandleMatcher( }, [&](NameRef* n) -> absl::StatusOr { // Comparing for equivalence to a (referenced) name. - auto* name_ref = std::get(leaf); - const auto* name_def = - std::get(name_ref->name_def()); + const auto* name_def = std::get(n->name_def()); XLS_ASSIGN_OR_RETURN(BValue to_match, Use(name_def)); - BValue result = Def(matcher, [&](const SourceInfo& loc) { + BValue result = Def(matcher_node, [&](const SourceInfo& loc) { return function_builder_->Eq(to_match, matched_value); }); - XLS_RETURN_IF_ERROR(DefAlias(name_def, name_ref)); + XLS_RETURN_IF_ERROR(DefAlias(name_def, n)); return result; }, [&](NameDef* name_def) -> absl::StatusOr { BValue ok = Def(name_def, [&](const SourceInfo& loc) { return function_builder_->Literal(UBits(1, 1)); }); - SetNodeToIr(matcher, matched_value); - SetNodeToIr(ToAstNode(leaf), matched_value); + SetNodeToIr(matcher_node, matched_value); return ok; }, + [&](TuplePattern*) -> absl::StatusOr { + return absl::InternalError("Tuple pattern reached leaf handler"); + }, }, - leaf); + matcher); } + TuplePattern* tuple_pattern = std::get(matcher); auto* matched_tuple_type = dynamic_cast(&matched_type); - XLS_ASSIGN_OR_RETURN((auto [number_of_tuple_elements, number_of_names]), - GetTupleSizes(matcher, matched_tuple_type)); + XLS_ASSIGN_OR_RETURN( + (auto [number_of_tuple_elements, non_rest_pattern_member_count]), + GetTupleSizes(tuple_pattern, matched_tuple_type)); int64_t tuple_index = 0; BValue ok = function_builder_->Literal(UBits(/*value=*/1, /*bit_count=*/1)); - const NameDefTree::Nodes& nodes = matcher->nodes(); - for (int64_t name_index = 0; name_index < nodes.size(); ++name_index) { - NameDefTree* element = nodes[name_index]; - if (element->IsRestOfTupleLeaf()) { + const std::vector& members = tuple_pattern->members(); + for (int64_t member_index = 0; member_index < members.size(); + ++member_index) { + const PatternTree& element = members[member_index]; + if (IsRestOfTupleLeaf(element)) { // Skip ahead. - int64_t wildcards_to_insert = number_of_tuple_elements - number_of_names; + int64_t wildcards_to_insert = + number_of_tuple_elements - non_rest_pattern_member_count; tuple_index += wildcards_to_insert; continue; } diff --git a/xls/dslx/ir_convert/function_converter.h b/xls/dslx/ir_convert/function_converter.h index f894a77e85..9c29eb235a 100644 --- a/xls/dslx/ir_convert/function_converter.h +++ b/xls/dslx/ir_convert/function_converter.h @@ -421,7 +421,7 @@ class FunctionConverter { // handles the `ivar` named `i`. absl::StatusOr HandleRangedForInductionVariable( const For* node, FunctionConverter& body_converter, - NameDefTree::Leaf ivar); + const PatternTree& ivar); // Helpers that adds a parameter for the loop carried accumulator value. // Handles the fact that we may need to destructure a pattern for @@ -429,7 +429,7 @@ class FunctionConverter { absl::Status HandleForLoopCarry(const For* node, FunctionConverter& body_converter, const std::optional& range_data, - BValue loop_index, NameDefTree::Leaf ivar, + BValue loop_index, const PatternTree& ivar, AstNode* carry_node); // Returns the relevant name definitions for the lexical scope of the for @@ -488,7 +488,7 @@ class FunctionConverter { absl::Status HandleCoverBuiltin(const Invocation* node, BValue condition); // Handles an arm of a match expression. - absl::StatusOr HandleMatcher(NameDefTree* matcher, + absl::StatusOr HandleMatcher(const PatternTree& matcher, const BValue& matched_value, const Type& matched_type); diff --git a/xls/dslx/ir_convert/proc_config_ir_converter.cc b/xls/dslx/ir_convert/proc_config_ir_converter.cc index a86a9c602b..e5bb6db6b1 100644 --- a/xls/dslx/ir_convert/proc_config_ir_converter.cc +++ b/xls/dslx/ir_convert/proc_config_ir_converter.cc @@ -196,7 +196,7 @@ absl::Status ProcConfigIrConverter::HandleLet(const Let* node) { XLS_RETURN_IF_ERROR(node->rhs()->Accept(this)); if (ChannelDecl* decl = dynamic_cast(node->rhs())) { - std::vector leaves = node->name_def_tree()->Flatten(); + std::vector leaves = FlattenPattern(node->pattern()); XLS_RET_CHECK_EQ(leaves.size(), 2); for (int i = 0; i < 2; i++) { if (std::holds_alternative(leaves[i])) { @@ -213,13 +213,13 @@ absl::Status ProcConfigIrConverter::HandleLet(const Let* node) { std::holds_alternative(leaves[i])); } } else { - if (!node->name_def_tree()->is_leaf()) { + if (std::holds_alternative(node->pattern())) { return absl::UnimplementedError( "Destructuring let bindings are not yet supported in Proc configs."); } // A leaf on the LHS of a Let will always be a NameDef. - NameDef* def = std::get(node->name_def_tree()->leaf()); + NameDef* def = std::get(node->pattern()); if (!node_to_ir_.contains(node->rhs())) { return absl::InternalError( absl::StrCat("Let RHS not evaluated as constexpr: ", def->ToString(), diff --git a/xls/dslx/lsp/language_server_adapter.cc b/xls/dslx/lsp/language_server_adapter.cc index 099ada0537..bd1f29a138 100644 --- a/xls/dslx/lsp/language_server_adapter.cc +++ b/xls/dslx/lsp/language_server_adapter.cc @@ -347,21 +347,23 @@ LanguageServerAdapter::InlayHint(LspUri uri, // Already has a type annotated, no need for inlay. continue; } - const auto* name_def_tree = let->name_def_tree(); - std::optional maybe_type = type_info.GetItem(name_def_tree); + const PatternTree& pattern = let->pattern(); + AstNode* pattern_node = ToAstNode(pattern); + std::optional maybe_type = type_info.GetItem(pattern_node); if (!maybe_type.has_value()) { // This can happen because we have a parametric function -- we don't // have concrete types because it could be instantiated in different // ways from different invocations. VLOG(5) << "InlayHint; no type information available for: " - << name_def_tree->ToString() << " @ " - << name_def_tree->span().ToString(file_table) << " within `" + << PatternToString(pattern) << " @ " + << GetPatternSpan(pattern).ToString(file_table) << " within `" << let->ToString() << "`"; continue; } const Type& type = *maybe_type.value(); results.push_back(verible::lsp::InlayHint{ - .position = ConvertPosToLspPosition(name_def_tree->span().limit()), + .position = + ConvertPosToLspPosition(GetPatternSpan(pattern).limit()), .label = absl::StrCat(": ", type.ToInlayHintString()), .kind = verible::lsp::InlayHintKind::kType, .paddingRight = true, diff --git a/xls/dslx/lsp/language_server_adapter_test.cc b/xls/dslx/lsp/language_server_adapter_test.cc index c86757a990..f395caed6b 100644 --- a/xls/dslx/lsp/language_server_adapter_test.cc +++ b/xls/dslx/lsp/language_server_adapter_test.cc @@ -20,8 +20,6 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/status_matchers.h" @@ -29,6 +27,8 @@ #include "absl/strings/match.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" #include "verible/common/lsp/lsp-file-utils.h" #include "verible/common/lsp/lsp-protocol.h" #include "xls/common/file/filesystem.h" @@ -313,6 +313,30 @@ TEST(LanguageServerAdapterTest, InlayHintForLetStatement) { EXPECT_EQ(hint.label, ": uN[32]"); } +TEST(LanguageServerAdapterTest, InlayHintForTupleDestructuringLetStatement) { + LanguageServerAdapter adapter(GetDslxStdlibUri(), /*dslx_paths=*/{}); + const LspUri kUri("file:///fake/path/test.x"); + XLS_ASSERT_OK(adapter.Update(kUri, R"(fn f(x: u32, y: u32) -> u32 { + let (a, b) = (x, y); + a + b +})")); + + const auto kInputRange = + verible::lsp::Range{.start = verible::lsp::Position{1, 0}, + .end = verible::lsp::Position{2, 0}}; + XLS_ASSERT_OK_AND_ASSIGN(std::vector hints, + adapter.InlayHint(kUri, kInputRange)); + + ASSERT_EQ(hints.size(), 1); + + const verible::lsp::InlayHint& hint = hints.at(0); + verible::lsp::Position want_position{1, 12}; + EXPECT_THAT(hint.position, PosEq(want_position)) + << "got: " << DebugString(hint.position) + << " want: " << DebugString(want_position); + EXPECT_EQ(hint.label, ": (uN[32], uN[32])"); +} + // Shows that we do not get inlay hints for parametric functions. TEST(LanguageServerAdapterTest, InlayHintForDistinctRoutine) { LanguageServerAdapter adapter(GetDslxStdlibUri(), /*dslx_paths=*/{}); diff --git a/xls/dslx/type_system/deduce_utils.cc b/xls/dslx/type_system/deduce_utils.cc index 6b830c37d9..888b5cfc0a 100644 --- a/xls/dslx/type_system/deduce_utils.cc +++ b/xls/dslx/type_system/deduce_utils.cc @@ -375,14 +375,14 @@ absl::Status TypeOrAnnotationErrorStatus(Span span, TupleTypeOrAnnotation type, } absl::StatusOr> GetTupleSizes( - const NameDefTree* name_def_tree, TupleTypeOrAnnotation tuple_type) { - const FileTable& file_table = *name_def_tree->owner()->file_table(); + const TuplePattern* tuple_pattern, TupleTypeOrAnnotation tuple_type) { + const FileTable& file_table = *tuple_pattern->owner()->file_table(); bool rest_of_tuple_found = false; - for (const NameDefTree* node : name_def_tree->nodes()) { - if (node->IsRestOfTupleLeaf()) { + for (const PatternTree& member : tuple_pattern->members()) { + if (IsRestOfTupleLeaf(member)) { if (rest_of_tuple_found) { return TypeOrAnnotationErrorStatus( - node->span(), tuple_type, + GetPatternSpan(member), tuple_type, absl::StrFormat("`..` can only be used once per tuple pattern."), file_table); } @@ -390,23 +390,26 @@ absl::StatusOr> GetTupleSizes( } } int64_t number_of_tuple_elements = Size(tuple_type); - int64_t number_of_names = name_def_tree->nodes().size(); - bool number_mismatch = number_of_names != number_of_tuple_elements; + int64_t non_rest_pattern_member_count = tuple_pattern->members().size(); + bool number_mismatch = + non_rest_pattern_member_count != number_of_tuple_elements; if (rest_of_tuple_found) { - // There's a "rest of tuple" in the name def tree; we only need to have - // enough tuple elements to bind to the required names. + // There's a "rest of tuple" in the pattern; we only need to have + // enough tuple elements for the non-rest pattern members. // Subtract 1 for the ".." - number_of_names--; - number_mismatch = number_of_names > number_of_tuple_elements; + non_rest_pattern_member_count--; + number_mismatch = non_rest_pattern_member_count > number_of_tuple_elements; } if (number_mismatch) { return TypeOrAnnotationErrorStatus( - name_def_tree->span(), tuple_type, + tuple_pattern->span(), tuple_type, absl::StrFormat("Cannot match a %d-element tuple to %d values.", - number_of_tuple_elements, number_of_names), + number_of_tuple_elements, + non_rest_pattern_member_count), file_table); } - return std::make_pair(number_of_tuple_elements, number_of_names); + return std::make_pair(number_of_tuple_elements, + non_rest_pattern_member_count); } static absl::StatusOr GetTupleType( @@ -439,43 +442,47 @@ static TypeOrAnnotation GetSubType(TupleTypeOrAnnotation type, int64_t index) { return std::get(type)->members()[index]; } -absl::Status MatchTupleNodeToType( +absl::Status MatchPatternToType( std::function)> - process_tuple_member, - const NameDefTree* name_def_tree, const TypeOrAnnotation type, + process_pattern_node, + const PatternTree& pattern, const TypeOrAnnotation type, const FileTable& file_table, std::optional constexpr_value) { - if (name_def_tree->is_leaf()) { - AstNode* name_def = ToAstNode(name_def_tree->leaf()); - XLS_RETURN_IF_ERROR(process_tuple_member(name_def, type, constexpr_value)); + if (!std::holds_alternative(pattern)) { + XLS_RETURN_IF_ERROR( + process_pattern_node(ToAstNode(pattern), type, constexpr_value)); return absl::OkStatus(); } + const TuplePattern* tuple_pattern = std::get(pattern); XLS_ASSIGN_OR_RETURN(TupleTypeOrAnnotation tuple_type, - GetTupleType(type, name_def_tree->span(), file_table)); + GetTupleType(type, tuple_pattern->span(), file_table)); - XLS_ASSIGN_OR_RETURN((auto [number_of_tuple_elements, number_of_names]), - GetTupleSizes(name_def_tree, tuple_type)); + XLS_ASSIGN_OR_RETURN( + (auto [number_of_tuple_elements, non_rest_pattern_member_count]), + GetTupleSizes(tuple_pattern, tuple_type)); // Index into the current tuple type. int64_t tuple_index = 0; - // Must iterate through the actual nodes size, not number_of_names, because - // there may be a "rest of tuple" leaf which decreases the number of names. - for (int64_t name_index = 0; name_index < name_def_tree->nodes().size(); - ++name_index) { - NameDefTree* subtree = name_def_tree->nodes()[name_index]; - if (subtree->IsRestOfTupleLeaf()) { + // Must iterate through the actual member count, not + // non_rest_pattern_member_count, because there may be a "rest of tuple" + // leaf which decreases the non-rest pattern member count. + for (int64_t member_index = 0; member_index < tuple_pattern->members().size(); + ++member_index) { + const PatternTree& subtree = tuple_pattern->members()[member_index]; + if (IsRestOfTupleLeaf(subtree)) { // Skip ahead. - tuple_index += number_of_tuple_elements - number_of_names; + tuple_index += number_of_tuple_elements - non_rest_pattern_member_count; continue; } TypeOrAnnotation subtype = GetSubType(tuple_type, tuple_index); - XLS_RETURN_IF_ERROR(process_tuple_member(subtree, subtype, std::nullopt)); + XLS_RETURN_IF_ERROR( + process_pattern_node(ToAstNode(subtree), subtype, std::nullopt)); std::optional sub_value; if (constexpr_value.has_value()) { sub_value = constexpr_value.value().GetValuesOrDie()[tuple_index]; } - XLS_RETURN_IF_ERROR(MatchTupleNodeToType(process_tuple_member, subtree, - subtype, file_table, sub_value)); + XLS_RETURN_IF_ERROR(MatchPatternToType(process_pattern_node, subtree, + subtype, file_table, sub_value)); ++tuple_index; } @@ -687,8 +694,8 @@ absl::StatusOr GetConfiguredValueAsInterpValue( std::string PatternsToString(const MatchArm* arm) { return absl::StrJoin(arm->patterns(), " | ", - [](std::string* out, NameDefTree* ndt) { - absl::StrAppend(out, ndt->ToString()); + [](std::string* out, const PatternTree& pattern) { + absl::StrAppend(out, PatternToString(pattern)); }); } diff --git a/xls/dslx/type_system/deduce_utils.h b/xls/dslx/type_system/deduce_utils.h index 9b344bf8f9..5271685069 100644 --- a/xls/dslx/type_system/deduce_utils.h +++ b/xls/dslx/type_system/deduce_utils.h @@ -91,40 +91,41 @@ absl::StatusOr ResolveBitSliceIndices( int64_t bit_count, std::optional start_opt, std::optional limit_opt); -// Checks that the number of tuple elements in the name def tree matches the +// Checks that the number of tuple elements in the tuple pattern matches the // number of tuple elements in the type; if a "rest of tuple" leaf is -// present, only one is allowed, and it is not counted in the number of names. +// present, only one is allowed, and it is excluded from the non-rest pattern +// member count. // -// Returns the number of tuple elements (first) and the number of names that -// will be bound in the given NameDefTree (second). +// Returns the number of tuple elements (first) and the number of top-level +// pattern members excluding rest-of-tuple (second). // // The latter may be less than the former if there is a "rest of tuple" leaf. using TupleTypeOrAnnotation = std::variant; absl::StatusOr> GetTupleSizes( - const NameDefTree* name_def_tree, TupleTypeOrAnnotation tuple_type); + const TuplePattern* tuple_pattern, TupleTypeOrAnnotation tuple_type); -// Typechecks the name def tree items against type, and recursively processes -// the node/type pairs according to the `process_tuple_member` function. -// If `constexpr_value` is provided for the tuple, the appropriate -// subvalue will also be passed into `process_tuple_member`. +// Typechecks the pattern items against type, and recursively processes +// the node/type pairs according to the `process_pattern_node` function. +// If `constexpr_value` is provided for the pattern, the appropriate +// subvalue will also be passed into `process_pattern_node`. // // For example: // // (a, (b, c)) vs (u8, (u4, u2)) // -// Will call `process_tuple_member` with the following arguments: +// Will call `process_pattern_node` with the following arguments: // // (a, u8, ...) // (b, u4, ...) // (c, u2, ...) // using TypeOrAnnotation = std::variant; -absl::Status MatchTupleNodeToType( +absl::Status MatchPatternToType( std::function)> - process_tuple_member, - const NameDefTree* name_def_tree, TypeOrAnnotation type, + process_pattern_node, + const PatternTree& pattern, TypeOrAnnotation type, const FileTable& file_table, std::optional constexpr_value); // Returns true if the cast-conversion from "from" to "to" is acceptable (i.e. diff --git a/xls/dslx/type_system/type_info.proto b/xls/dslx/type_system/type_info.proto index c1195e93ce..8af524e991 100644 --- a/xls/dslx/type_system/type_info.proto +++ b/xls/dslx/type_system/type_info.proto @@ -43,6 +43,7 @@ enum AstNodeKindProto { AST_NODE_KIND_ARRAY = 18; AST_NODE_KIND_STRING = 19; AST_NODE_KIND_STRUCT_INSTANCE = 20; + // Retired legacy wire value; do not reuse. AST_NODE_KIND_NAME_DEF_TREE = 21; AST_NODE_KIND_SPLAT_STRUCT_INSTANCE = 22; AST_NODE_KIND_INDEX = 23; @@ -98,6 +99,7 @@ enum AstNodeKindProto { AST_NODE_KIND_TRAIT = 73; AST_NODE_KIND_ATTRIBUTE = 74; AST_NODE_KIND_FUZZ_TEST_FUNCTION = 75; + AST_NODE_KIND_TUPLE_PATTERN = 76; } message BitsValueProto { diff --git a/xls/dslx/type_system/type_info_to_proto.cc b/xls/dslx/type_system/type_info_to_proto.cc index 06b8057493..d15415f16a 100644 --- a/xls/dslx/type_system/type_info_to_proto.cc +++ b/xls/dslx/type_system/type_info_to_proto.cc @@ -101,8 +101,8 @@ AstNodeKindProto ToProto(AstNodeKind kind) { return AST_NODE_KIND_STRUCT_INSTANCE; case AstNodeKind::kStructMember: return AST_NODE_KIND_STRUCT_MEMBER; - case AstNodeKind::kNameDefTree: - return AST_NODE_KIND_NAME_DEF_TREE; + case AstNodeKind::kTuplePattern: + return AST_NODE_KIND_TUPLE_PATTERN; case AstNodeKind::kSplatStructInstance: return AST_NODE_KIND_SPLAT_STRUCT_INSTANCE; case AstNodeKind::kIndex: @@ -718,7 +718,11 @@ absl::StatusOr FromProto(AstNodeKindProto p) { case AST_NODE_KIND_STRUCT_MEMBER: return AstNodeKind::kStructMember; case AST_NODE_KIND_NAME_DEF_TREE: - return AstNodeKind::kNameDefTree; + return absl::InvalidArgumentError( + "Legacy NameDefTree type-info entries are unsupported; re-typecheck " + "the module"); + case AST_NODE_KIND_TUPLE_PATTERN: + return AstNodeKind::kTuplePattern; case AST_NODE_KIND_SPLAT_STRUCT_INSTANCE: return AstNodeKind::kSplatStructInstance; case AST_NODE_KIND_INDEX: diff --git a/xls/dslx/type_system/type_info_to_proto_test.cc b/xls/dslx/type_system/type_info_to_proto_test.cc index 5c4f85f338..c5c227907b 100644 --- a/xls/dslx/type_system/type_info_to_proto_test.cc +++ b/xls/dslx/type_system/type_info_to_proto_test.cc @@ -19,9 +19,9 @@ #include #include +#include "absl/strings/str_format.h" #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "absl/strings/str_format.h" #include "re2/re2.h" #include "xls/common/golden_files.h" #include "xls/common/status/matchers.h" @@ -192,5 +192,38 @@ fn bit_update() -> u8 { EXPECT_THAT(nodes_text, ::testing::Not(::testing::HasSubstr(""))); } +TEST_F(TypeInfoToProtoWithBothTypecheckVersionsTest, + TuplePatternUsesDistinctAstNodeKind) { + ImportData import_data = CreateImportDataForTest(); + XLS_ASSERT_OK_AND_ASSIGN( + TypecheckedModule tm, + ParseAndTypecheck("fn f() -> u32 { let (x, y) = (u32:1, u32:2); x }", + "fake.x", "fake", &import_data, nullptr)); + XLS_ASSERT_OK_AND_ASSIGN(TypeInfoProto tip, + TypeInfoToProto(*tm.type_info, tm.module)); + XLS_ASSERT_OK(ToHumanString(tip, import_data, import_data.file_table())); + + bool found_tuple_pattern = false; + for (const AstNodeTypeInfoProto& node : tip.nodes()) { + found_tuple_pattern |= node.kind() == AST_NODE_KIND_TUPLE_PATTERN; + EXPECT_NE(node.kind(), AST_NODE_KIND_NAME_DEF_TREE); + } + EXPECT_TRUE(found_tuple_pattern); +} + +TEST_F(TypeInfoToProtoWithBothTypecheckVersionsTest, + RejectsLegacyNameDefTreeAstNodeKind) { + ImportData import_data = CreateImportDataForTest(); + AstNodeTypeInfoProto legacy; + legacy.set_kind(AST_NODE_KIND_NAME_DEF_TREE); + legacy.mutable_type()->mutable_token_type(); + + EXPECT_THAT( + ToHumanString(legacy, import_data, import_data.file_table()), + absl_testing::StatusIs( + absl::StatusCode::kInvalidArgument, + ::testing::HasSubstr("Legacy NameDefTree type-info entries"))); +} + } // namespace } // namespace xls::dslx diff --git a/xls/dslx/type_system/typecheck_module_test.cc b/xls/dslx/type_system/typecheck_module_test.cc index ad8e8a414f..19136df4ce 100644 --- a/xls/dslx/type_system/typecheck_module_test.cc +++ b/xls/dslx/type_system/typecheck_module_test.cc @@ -21,13 +21,13 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/container/flat_hash_map.h" #include "absl/status/status.h" #include "absl/status/status_matchers.h" #include "absl/status/statusor.h" #include "absl/strings/str_replace.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" #include "xls/common/status/matchers.h" #include "xls/common/status/status_macros.h" #include "xls/dslx/create_import_data.h" @@ -4525,10 +4525,10 @@ fn f(x: u8) -> u2 { Match* m = dynamic_cast(expr); ASSERT_NE(m, nullptr); const MatchArm* arm = m->arms()[0]; - const NameDefTree* pattern = arm->patterns()[0]; + const PatternTree& pattern = arm->patterns()[0]; // Check that the pattern is just a leaf NameRef. - NameRef* name_ref = std::get(pattern->leaf()); + NameRef* name_ref = std::get(pattern); ASSERT_NE(name_ref, nullptr); EXPECT_EQ(name_ref->identifier(), "FOO"); diff --git a/xls/dslx/type_system_v2/constant_collector.cc b/xls/dslx/type_system_v2/constant_collector.cc index c813e66608..67dab00553 100644 --- a/xls/dslx/type_system_v2/constant_collector.cc +++ b/xls/dslx/type_system_v2/constant_collector.cc @@ -327,7 +327,7 @@ class Visitor : public AstNodeVisitorWithDefault { } // Reminder: we don't allow name destructuring in constant defs, so this // is expected to never fail. - XLS_RET_CHECK_EQ(let->name_def_tree()->GetNameDefs().size(), 1); + XLS_RET_CHECK_EQ(GetPatternNameDefs(let->pattern()).size(), 1); } else if (!value.ok()) { return absl::OkStatus(); } @@ -342,8 +342,8 @@ class Visitor : public AstNodeVisitorWithDefault { } return absl::OkStatus(); }; - XLS_RETURN_IF_ERROR(MatchTupleNodeToType(note_members, let->name_def_tree(), - &type_, file_table_, *value)); + XLS_RETURN_IF_ERROR(MatchPatternToType(note_members, let->pattern(), &type_, + file_table_, *value)); return absl::OkStatus(); } @@ -401,21 +401,26 @@ class Visitor : public AstNodeVisitorWithDefault { // is just `i` or `acc`, but each of those is allowed to be a further // destructured tuple. absl::StatusOr ShadowConstForInput( - const NameDefTree* input, Expr* iteration_value, + const PatternTree& input, Expr* iteration_value, absl::flat_hash_map& old_to_new_name_defs) { absl::flat_hash_map pairs; + AstNode* input_node = ToAstNode(input); XLS_ASSIGN_OR_RETURN( - pairs, CloneAstAndGetAllPairs(input, input->owner(), + pairs, CloneAstAndGetAllPairs(input_node, input_node->owner(), &PreserveTypeDefinitionsReplacer)); - NameDefTree* iteration_ndt = absl::down_cast(pairs.at(input)); + PatternTree iteration_pattern = std::visit( + [&](auto* node) -> PatternTree { + return absl::down_cast(pairs.at(node)); + }, + input); for (const auto& [old_node, new_node] : pairs) { if (old_node->kind() == AstNodeKind::kNameDef) { old_to_new_name_defs.emplace(absl::down_cast(old_node), absl::down_cast(new_node)); } } - return module_.Make(input->span(), iteration_ndt, /*type=*/nullptr, - iteration_value, + return module_.Make(GetPatternSpan(input), iteration_pattern, + /*type=*/nullptr, iteration_value, /*is_const=*/false); } @@ -444,21 +449,24 @@ class Visitor : public AstNodeVisitorWithDefault { TypeSystemTrace trace = tracer_.TraceUnroll(expr); std::vector unrolled_statements; - CHECK_EQ(expr->names()->nodes().size(), 2); - NameDefTree* iterator_name = expr->names()->nodes()[0]; - NameDefTree* accumulator_name = expr->names()->nodes()[1]; + CHECK(std::holds_alternative(expr->pattern())); + const TuplePattern* tuple_pattern = + std::get(expr->pattern()); + CHECK_EQ(tuple_pattern->members().size(), 2); + const PatternTree& iterator_pattern = tuple_pattern->members()[0]; + const PatternTree& accumulator_pattern = tuple_pattern->members()[1]; TypeAnnotation* iterator_type = nullptr; TypeAnnotation* accumulator_type = nullptr; - std::optional name_type_annotation = - table_.GetTypeAnnotation(expr->names()); - if (name_type_annotation.has_value()) { + std::optional pattern_type_annotation = + table_.GetTypeAnnotation(ToAstNode(expr->pattern())); + if (pattern_type_annotation.has_value()) { // Note that common type checking for ForLoopBase should verify this // before it gets here. XLS_RET_CHECK( - (*name_type_annotation)->IsAnnotation()); + (*pattern_type_annotation)->IsAnnotation()); const auto* types = - (*name_type_annotation)->AsAnnotation(); + (*pattern_type_annotation)->AsAnnotation(); CHECK_EQ(types->size(), 2); iterator_type = types->members()[0]; accumulator_type = types->members()[1]; @@ -489,20 +497,20 @@ class Visitor : public AstNodeVisitorWithDefault { } bool has_result_value = - !accumulator_name->IsWildcardLeaf() && !expr->body()->trailing_semi(); + !IsWildcardLeaf(accumulator_pattern) && !expr->body()->trailing_semi(); Expr* accumulator_value = expr->init(); for (uint64_t i = 0; i < size; i++) { absl::flat_hash_map iteration_name_def_mapping; if (has_result_value) { XLS_ASSIGN_OR_RETURN( Let * accumulator, - ShadowConstForInput(accumulator_name, accumulator_value, + ShadowConstForInput(accumulator_pattern, accumulator_value, iteration_name_def_mapping)); XLS_RETURN_IF_ERROR( table_.SetTypeAnnotation(accumulator, accumulator_type)); unrolled_statements.push_back(module_.Make(accumulator)); } - if (!iterator_name->IsWildcardLeaf()) { + if (!IsWildcardLeaf(iterator_pattern)) { Let* iterator = nullptr; if (iterable_values && (*iterable_values)[i].FitsInUint64()) { Number* value = module_.Make( @@ -516,7 +524,7 @@ class Visitor : public AstNodeVisitorWithDefault { ti_->SetItem(value, MetaType(value_type->CloneToUnique())); XLS_ASSIGN_OR_RETURN(iterator, - ShadowConstForInput(iterator_name, value, + ShadowConstForInput(iterator_pattern, value, iteration_name_def_mapping)); } else { Expr* index = module_.Make( @@ -531,7 +539,7 @@ class Visitor : public AstNodeVisitorWithDefault { Expr* element = module_.Make(expr->iterable()->span(), expr->iterable(), index, false); XLS_ASSIGN_OR_RETURN(iterator, - ShadowConstForInput(iterator_name, element, + ShadowConstForInput(iterator_pattern, element, iteration_name_def_mapping)); } XLS_RETURN_IF_ERROR(table_.SetTypeAnnotation(iterator, iterator_type)); diff --git a/xls/dslx/type_system_v2/decorate_error.cc b/xls/dslx/type_system_v2/decorate_error.cc index c3b4800511..c9062e7b0a 100644 --- a/xls/dslx/type_system_v2/decorate_error.cc +++ b/xls/dslx/type_system_v2/decorate_error.cc @@ -97,24 +97,17 @@ bool IsBuiltinSendVariantInvocation(const Invocation* invocation) { name_ref->identifier() == "send_if"; } -bool IsInNameDefTreePosition(const AstNode* node, int index) { +bool IsInTuplePatternPosition(const AstNode* node, int index) { if (node->parent() == nullptr || - node->parent()->kind() != AstNodeKind::kNameDefTree) { + node->parent()->kind() != AstNodeKind::kTuplePattern) { return false; } - const AstNode* parent = node->parent(); - if (parent->parent() == nullptr || - parent->parent()->kind() != AstNodeKind::kNameDefTree) { - return false; - } - - const auto* tree = absl::down_cast(parent->parent()); - if (tree->is_leaf() || tree->nodes().size() <= index) { + const auto* pattern = absl::down_cast(node->parent()); + if (pattern->members().size() <= index) { return false; } - return tree->nodes().at(index)->is_leaf() && - ToAstNode(tree->nodes().at(index)->leaf()) == node; + return ToAstNode(pattern->members().at(index)) == node; } } // namespace @@ -188,7 +181,7 @@ absl::StatusOr DecorateError( if (!invocation->args().empty() && node == invocation->args()[0] && IsBuiltinSendVariantInvocation(invocation)) { absl::Status new_error = absl::OkStatus(); - if (IsInNameDefTreePosition(ToAstNode(arg_as_name_ref->name_def()), 1)) { + if (IsInTuplePatternPosition(ToAstNode(arg_as_name_ref->name_def()), 1)) { // The root cause is probably a `let (data, tok) = recv(...)` or // similar. Because we don't actually check what is on the right-hand // side of the let tuple, the error message is not worded with 100% diff --git a/xls/dslx/type_system_v2/flatten_in_type_order.cc b/xls/dslx/type_system_v2/flatten_in_type_order.cc index 0036aa970e..464184279e 100644 --- a/xls/dslx/type_system_v2/flatten_in_type_order.cc +++ b/xls/dslx/type_system_v2/flatten_in_type_order.cc @@ -104,10 +104,10 @@ class Flattener : public AstNodeVisitorWithDefault { } XLS_RETURN_IF_ERROR(node->rhs()->Accept(this)); nodes_.push_back(node); - for (const NameDef* name_def : node->name_def_tree()->GetNameDefs()) { + for (const NameDef* name_def : GetPatternNameDefs(node->pattern())) { XLS_RETURN_IF_ERROR(name_def->Accept(this)); } - nodes_.push_back(node->name_def_tree()); + nodes_.push_back(ToAstNode(node->pattern())); return absl::OkStatus(); } @@ -143,8 +143,8 @@ class Flattener : public AstNodeVisitorWithDefault { } absl::Status HandleMatchArm(const MatchArm* node) override { - for (const NameDefTree* name_def_tree : node->patterns()) { - XLS_RETURN_IF_ERROR(name_def_tree->Accept(this)); + for (const PatternTree& pattern : node->patterns()) { + XLS_RETURN_IF_ERROR(ToAstNode(pattern)->Accept(this)); } if (node->expr()->kind() == AstNodeKind::kStatementBlock) { // Statement blocks as arm exprs have special handling which essentially @@ -184,7 +184,7 @@ class Flattener : public AstNodeVisitorWithDefault { XLS_RETURN_IF_ERROR(node->type_annotation()->Accept(this)); } XLS_RETURN_IF_ERROR(node->iterable()->Accept(this)); - XLS_RETURN_IF_ERROR(node->names()->Accept(this)); + XLS_RETURN_IF_ERROR(ToAstNode(node->pattern())->Accept(this)); XLS_RETURN_IF_ERROR(node->init()->Accept(this)); nodes_.push_back(node); return absl::OkStatus(); diff --git a/xls/dslx/type_system_v2/inference_table_converter_impl.cc b/xls/dslx/type_system_v2/inference_table_converter_impl.cc index 0294b521ca..d001a8ab85 100644 --- a/xls/dslx/type_system_v2/inference_table_converter_impl.cc +++ b/xls/dslx/type_system_v2/inference_table_converter_impl.cc @@ -461,7 +461,7 @@ class InferenceTableConverterImpl : public InferenceTableConverter, // Dig up the actual declaration, e.g. the `let x = ...;` node for a node // like the `x` under that. const AstNode* decl = name_def->parent(); - while (decl != nullptr && decl->kind() == AstNodeKind::kNameDefTree && + while (decl != nullptr && decl->kind() == AstNodeKind::kTuplePattern && decl->parent() != nullptr) { decl = decl->parent(); } diff --git a/xls/dslx/type_system_v2/populate_table_visitor.cc b/xls/dslx/type_system_v2/populate_table_visitor.cc index 532f43defc..38c7478c0e 100644 --- a/xls/dslx/type_system_v2/populate_table_visitor.cc +++ b/xls/dslx/type_system_v2/populate_table_visitor.cc @@ -623,12 +623,9 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, XLS_RETURN_IF_ERROR(table_.SetTypeVariable(arm->expr(), arm_type)); } - for (const NameDefTree* pattern : arm->patterns()) { - XLS_RETURN_IF_ERROR(table_.SetTypeVariable(pattern, matched_var)); - if (pattern->is_leaf()) { - XLS_RETURN_IF_ERROR( - table_.SetTypeVariable(ToAstNode(pattern->leaf()), matched_var)); - } + for (const PatternTree& pattern : arm->patterns()) { + XLS_RETURN_IF_ERROR( + table_.SetTypeVariable(ToAstNode(pattern), matched_var)); } } if (node->IsConst()) { @@ -712,57 +709,53 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, return table_.SetTypeAnnotation(node, any_type); } - // Recursively handles a `NameDefTree`, propagating the unified type of the + // Recursively handles a tuple pattern, propagating the unified type of the // tree downward to children (i.e. the children's types are element types of // the tree's type variable). This function sets the root-level type to be a - // tuple of `Any` types, the rationale being that a `NameDefTree` itself does + // tuple of `Any` types, the rationale being that a tuple pattern itself does // not provide any top-level type information but the allowable size of the - // tuple. A `NameDefTree` must be used in a context where this tuple of `Any` + // tuple. A tuple pattern must be used in a context where this tuple of `Any` // will get unified against a source of actual type information. - absl::Status HandleNameDefTree(const NameDefTree* node) override { - VLOG(5) << "HandleNameDefTree: " << node->ToString(); - - if (node->is_leaf()) { - return DefaultHandler(node); - } - + absl::Status HandleTuplePattern(const TuplePattern* node) override { + VLOG(5) << "HandleTuplePattern: " << node->ToString(); std::vector member_types; const NameRef* variable = *table_.GetTypeVariable(node); - for (int i = 0; i < node->nodes().size(); i++) { - const NameDefTree* child = node->nodes()[i]; + for (int i = 0; i < node->members().size(); i++) { + const PatternTree& child = node->members()[i]; member_types.push_back( - module_.Make(child->IsRestOfTupleLeaf())); + module_.Make(IsRestOfTupleLeaf(child))); } TupleTypeAnnotation* tuple_annotation = module_.Make(node->span(), member_types); XLS_RETURN_IF_ERROR(table_.SetTypeAnnotation(node, tuple_annotation)); - return HandleNameDefTreeChildren( + return HandleTuplePatternChildren( node, module_.Make(variable)); } // Handles all the children of `subtree`, with `type` being the type of // `subtree` itself (a TVTA for unification of the whole tuple, or // user-specified type). - absl::Status HandleNameDefTreeChildren(const NameDefTree* subtree, - const TypeAnnotation* type) { + absl::Status HandleTuplePatternChildren(const TuplePattern* subtree, + const TypeAnnotation* type) { std::optional rest_of_tuple_index; - for (int i = 0; i < subtree->nodes().size(); i++) { - if (subtree->nodes()[i]->IsRestOfTupleLeaf()) { + for (int i = 0; i < subtree->members().size(); i++) { + if (IsRestOfTupleLeaf(subtree->members()[i])) { rest_of_tuple_index = i; break; } - XLS_RETURN_IF_ERROR(HandleNameDefTreeChild(subtree, type, i)); + XLS_RETURN_IF_ERROR(HandleTuplePatternChild(subtree, type, i)); } if (rest_of_tuple_index.has_value()) { - for (int i = subtree->nodes().size() - 1; i > *rest_of_tuple_index; i--) { - if (subtree->nodes()[i]->IsRestOfTupleLeaf()) { + for (int i = subtree->members().size() - 1; i > *rest_of_tuple_index; + i--) { + if (IsRestOfTupleLeaf(subtree->members()[i])) { return TypeInferenceErrorStatus( - subtree->nodes()[i]->span(), /*type=*/nullptr, + GetPatternSpan(subtree->members()[i]), /*type=*/nullptr, "`..` can only be used once per tuple pattern.", file_table_); } - XLS_RETURN_IF_ERROR(HandleNameDefTreeChild( + XLS_RETURN_IF_ERROR(HandleTuplePatternChild( subtree, type, i, /*use_right_based_index_in_type=*/true)); } } @@ -777,38 +770,40 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, // type variable on the child so that its parent-derived type will be unified // with any other information that traversing it picks up (e.g. if it is a // literal). - absl::Status HandleNameDefTreeChild( - const NameDefTree* tree, const TypeAnnotation* tree_type, int i, + absl::Status HandleTuplePatternChild( + const TuplePattern* tree, const TypeAnnotation* tree_type, int i, bool use_right_based_index_in_type = false) { - const NameDefTree* child = tree->nodes()[i]; - const AstNode* actual_child = - child->is_leaf() ? ToAstNode(child->leaf()) : child; + const PatternTree& child = tree->members()[i]; + const AstNode* actual_child = ToAstNode(child); const TypeAnnotation* element_type = nullptr; if (use_right_based_index_in_type) { // Index from the right uses element count - i. Expr* offset = CreateElementCountOffset( module_, const_cast(tree_type), - tree->nodes().size() - i); + tree->members().size() - i); XLS_RETURN_IF_ERROR(DefineAndSetTypeVariable(offset, "offset")); XLS_RETURN_IF_ERROR(table_.SetTypeAnnotation( - offset, CreateU32Annotation(module_, child->span()))); + offset, CreateU32Annotation(module_, GetPatternSpan(child)))); XLS_RETURN_IF_ERROR(offset->Accept(this)); element_type = module_.Make(tree_type, offset); } else { // Index from the left just uses a literal. XLS_ASSIGN_OR_RETURN( Number * index, - MakeTypeCheckedNumber(module_, table_, child->span(), i, - CreateU32Annotation(module_, child->span()))); + MakeTypeCheckedNumber( + module_, table_, GetPatternSpan(child), i, + CreateU32Annotation(module_, GetPatternSpan(child)))); element_type = module_.Make(tree_type, index); } - XLS_RETURN_IF_ERROR(DefineAndSetTypeVariable(actual_child, "ndt")); + XLS_RETURN_IF_ERROR( + DefineAndSetTypeVariable(actual_child, "pattern_child")); XLS_RETURN_IF_ERROR(table_.SetTypeAnnotation(actual_child, element_type)); - if (child->is_leaf()) { - XLS_RETURN_IF_ERROR(ToAstNode(child->leaf())->Accept(this)); + if (std::holds_alternative(child)) { + XLS_RETURN_IF_ERROR(HandleTuplePatternChildren( + std::get(child), element_type)); } else { - XLS_RETURN_IF_ERROR(HandleNameDefTreeChildren(child, element_type)); + XLS_RETURN_IF_ERROR(ToAstNode(child)->Accept(this)); } return absl::OkStatus(); } @@ -859,20 +854,22 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, table_.SetTypeAnnotation(node->iterable(), iterable_type_annotation)); } - // Handle namedef of iterator and accumulator. - if (!node->names()->IsIrrefutable() || node->names()->nodes().size() != 2) { + // Validate the iterator/accumulator binding pattern. + if (!IsIrrefutablePattern(node->pattern()) || + !std::holds_alternative(node->pattern()) || + std::get(node->pattern())->members().size() != 2) { return TypeInferenceErrorStatus( - node->names()->span(), + GetPatternSpan(node->pattern()), /*type=*/nullptr, - absl::Substitute("For-loop iterator and accumulator name tuple must " - "contain 2 top-level elements; got: `$0`", - node->names()->ToString()), + absl::Substitute("For-loop iterator and accumulator pattern must be " + "an irrefutable 2-element tuple; got: `$0`", + PatternToString(node->pattern())), file_table_); } - NameDefTree* iterator_ndt = node->names()->nodes()[0]; - AstNode* iterator = iterator_ndt->is_leaf() - ? ToAstNode(iterator_ndt->leaf()) - : iterator_ndt; + const TuplePattern* tuple_pattern = + std::get(node->pattern()); + const PatternTree& iterator_pattern = tuple_pattern->members()[0]; + AstNode* iterator = ToAstNode(iterator_pattern); XLS_ASSIGN_OR_RETURN( const NameRef* iterator_type_variable, DefineTypeVariable( @@ -885,15 +882,14 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, // The type of iterator and accumulator should be covariant with iterable's // element type and For node type respectively. const NameRef* for_node_type_variable = *table_.GetTypeVariable(node); - NameDefTree* accumulator_ndt = node->names()->nodes()[1]; - AstNode* accumulator = accumulator_ndt->is_leaf() - ? ToAstNode(accumulator_ndt->leaf()) - : accumulator_ndt; + const PatternTree& accumulator_pattern = tuple_pattern->members()[1]; + AstNode* accumulator = ToAstNode(accumulator_pattern); - if (node->body()->trailing_semi() && !accumulator_ndt->IsWildcardLeaf() && - !accumulator_ndt->IsRestOfTupleLeaf() && accumulator_ndt->is_leaf()) { + if (node->body()->trailing_semi() && !IsWildcardLeaf(accumulator_pattern) && + !IsRestOfTupleLeaf(accumulator_pattern) && + !std::holds_alternative(accumulator_pattern)) { return TypeInferenceErrorStatus( - accumulator_ndt->span(), /*type=*/nullptr, + GetPatternSpan(accumulator_pattern), /*type=*/nullptr, "Loop has an accumulator but the body does not produce a value. The " "semicolon at the end of the last body statement may be unintended.", file_table_); @@ -931,13 +927,14 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, module_.Make(for_node_type_variable))); XLS_RETURN_IF_ERROR(table_.SetTypeAnnotation( - node->names(), + ToAstNode(node->pattern()), module_.Make( - node->names()->span(), std::vector{ - module_.Make( - iterator_type_variable), - module_.Make( - for_node_type_variable)}))); + GetPatternSpan(node->pattern()), + std::vector{ + module_.Make( + iterator_type_variable), + module_.Make( + for_node_type_variable)}))); XLS_RETURN_IF_ERROR(node->iterable()->Accept(this)); XLS_RETURN_IF_ERROR(node->init()->Accept(this)); @@ -1896,15 +1893,11 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, XLS_RETURN_IF_ERROR(table_.SetTypeVariable(node->rhs(), variable)); XLS_RETURN_IF_ERROR(table_.SetTypeVariable(node, variable)); XLS_RETURN_IF_ERROR( - table_.SetTypeVariable(node->name_def_tree(), variable)); + table_.SetTypeVariable(ToAstNode(node->pattern()), variable)); if (node->type_annotation() != nullptr) { XLS_RETURN_IF_ERROR( table_.SetTypeAnnotation(node, node->type_annotation())); } - if (node->name_def_tree()->is_leaf()) { - XLS_RETURN_IF_ERROR(table_.SetTypeVariable( - ToAstNode(node->name_def_tree()->leaf()), variable)); - } return DefaultHandler(node); } diff --git a/xls/dslx/type_system_v2/typecheck_module_v2_array_tuple_test.cc b/xls/dslx/type_system_v2/typecheck_module_v2_array_tuple_test.cc index 7b8e3e4afb..14630d87b6 100644 --- a/xls/dslx/type_system_v2/typecheck_module_v2_array_tuple_test.cc +++ b/xls/dslx/type_system_v2/typecheck_module_v2_array_tuple_test.cc @@ -1239,8 +1239,8 @@ fn foo() { } )", TypecheckFails( - HasSubstr("For-loop iterator and accumulator name tuple " - "must contain 2 top-level elements; got: `(i, a, _)`"))); + HasSubstr("For-loop iterator and accumulator pattern must be an " + "irrefutable 2-element tuple; got: `(i, a, _)`"))); } TEST(TypecheckV2Test, UnrollForTupleTypeMismatch) { @@ -1251,8 +1251,8 @@ fn foo() { } )", TypecheckFails( - HasSubstr("For-loop iterator and accumulator name tuple must contain " - "2 top-level elements; got: `(i)`"))); + HasSubstr("For-loop iterator and accumulator pattern must be an " + "irrefutable 2-element tuple; got: `(i)`"))); } TEST(TypecheckV2Test, ProcWithChannelArray) { diff --git a/xls/dslx/type_system_v2/typecheck_module_v2_error_handler_test.cc b/xls/dslx/type_system_v2/typecheck_module_v2_error_handler_test.cc index b9e7486af8..19045643f7 100644 --- a/xls/dslx/type_system_v2/typecheck_module_v2_error_handler_test.cc +++ b/xls/dslx/type_system_v2/typecheck_module_v2_error_handler_test.cc @@ -17,8 +17,6 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/algorithm/container.h" #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" @@ -26,6 +24,8 @@ #include "absl/status/statusor.h" #include "absl/strings/str_join.h" #include "absl/types/span.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" #include "xls/common/status/matchers.h" #include "xls/dslx/frontend/ast.h" #include "xls/dslx/frontend/ast_node.h" @@ -258,7 +258,7 @@ fn f() { "Definition of `a` (type `uN[32]`) is not used in function `f`"); } -TEST(TypecheckV2Test, AllowSomeUnusedDefInNameDefTree) { +TEST(TypecheckV2Test, AllowSomeUnusedDefInTuplePattern) { XLS_ASSERT_OK_AND_ASSIGN(TypecheckResult result, TypecheckV2(R"( fn f(A: (u32, u32)[5]) -> (u32, u32) { for (i, (a, b)) in A { diff --git a/xls/dslx/type_system_v2/validate_concrete_type.cc b/xls/dslx/type_system_v2/validate_concrete_type.cc index 9c76f14c8f..4b7bd5f347 100644 --- a/xls/dslx/type_system_v2/validate_concrete_type.cc +++ b/xls/dslx/type_system_v2/validate_concrete_type.cc @@ -377,12 +377,12 @@ class TypeValidator : public AstNodeVisitorWithDefault { } for (MatchArm* arm : node->arms()) { - for (NameDefTree* pattern : arm->patterns()) { + for (const PatternTree& pattern : arm->patterns()) { bool exhaustive_before = exhaustiveness_checker.IsExhaustive(); - exhaustiveness_checker.AddPattern(*pattern); + exhaustiveness_checker.AddPattern(pattern); if (exhaustive_before) { warning_collector_.Add( - pattern->span(), WarningKind::kAlreadyExhaustiveMatch, + GetPatternSpan(pattern), WarningKind::kAlreadyExhaustiveMatch, "Match is already exhaustive before this pattern"); } } diff --git a/xls/fuzzer/ast_generator.cc b/xls/fuzzer/ast_generator.cc index 99a0cd76f7..237af0cbbb 100644 --- a/xls/fuzzer/ast_generator.cc +++ b/xls/fuzzer/ast_generator.cc @@ -925,7 +925,7 @@ absl::StatusOr AstGenerator::GenerateExprOfType( BitsAndSignedness{GetTypeBitCount(type), BitsTypeIsSigned(type).value()}); } -absl::StatusOr AstGenerator::GenerateMatchArmPattern( +absl::StatusOr AstGenerator::GenerateMatchArmPattern( Context* ctx, const TypeAnnotation* type) { XLS_RET_CHECK(!IsTypeRef(type)) << "Matched-on TypeRefs-typed values are not " "supported by the fuzzer; got: " @@ -937,7 +937,7 @@ absl::StatusOr AstGenerator::GenerateMatchArmPattern( // Ten percent of the time, generate a wildcard pattern. if (RandomBool(0.1)) { WildcardPattern* wc = module_->Make(fake_span_); - return module_->Make(fake_span_, wc); + return PatternTree{wc}; } // Ten percent of tuples should have a "rest of tuple", and skip @@ -947,14 +947,14 @@ absl::StatusOr AstGenerator::GenerateMatchArmPattern( RandomIntWithExpectedValue(tuple_type->size() / 2.0, 0); auto number_to_skip = RandomIntWithExpectedValue(tuple_type->size() / 2.0, 0); - std::vector tuple_values; - tuple_values.reserve(tuple_type->size() + 1); + std::vector pattern_members; + pattern_members.reserve(tuple_type->size() + 1); for (int64_t index = 0; index < tuple_type->size(); ++index) { if (rest_of_tuple_index == index && insert_rest_of_tuple) { insert_rest_of_tuple = false; RestOfTuple* rest = module_->Make(fake_span_); - tuple_values.push_back(module_->Make(fake_span_, rest)); + pattern_members.push_back(rest); // Jump forward a random # of elements if (number_to_skip > 0) { @@ -966,11 +966,12 @@ absl::StatusOr AstGenerator::GenerateMatchArmPattern( } XLS_ASSIGN_OR_RETURN( - NameDefTree * pattern, + PatternTree pattern, GenerateMatchArmPattern(ctx, tuple_type->members()[index])); - tuple_values.push_back(pattern); + pattern_members.push_back(pattern); } - return module_->Make(fake_span_, tuple_values); + return PatternTree{ + module_->Make(fake_span_, std::move(pattern_members))}; } CHECK(IsBits(type)) @@ -980,7 +981,7 @@ absl::StatusOr AstGenerator::GenerateMatchArmPattern( // Five percent of the time, generate a wildcard pattern. if (RandomBool(0.05)) { WildcardPattern* wc = module_->Make(fake_span_); - return module_->Make(fake_span_, wc); + return PatternTree{wc}; } // Fifteen percent of the time, generate a range. (Note that this is an @@ -1022,7 +1023,7 @@ absl::StatusOr AstGenerator::GenerateMatchArmPattern( } Range* range = module_->Make(fake_span_, start_type_expr.expr, inclusive_end, limit_type_expr.expr); - return module_->Make(fake_span_, range); + return PatternTree{range}; } // Rest of the time we generate a simple number as the pattern to match. @@ -1030,7 +1031,7 @@ absl::StatusOr AstGenerator::GenerateMatchArmPattern( BitsAndSignedness{GetTypeBitCount(type), BitsTypeIsSigned(type).value()}); Number* number = dynamic_cast(type_expr.expr); CHECK_NE(number, nullptr); - return module_->Make(fake_span_, number); + return PatternTree{number}; } absl::StatusOr AstGenerator::GenerateMatch(Context* ctx) { @@ -1057,20 +1058,19 @@ absl::StatusOr AstGenerator::GenerateMatch(Context* ctx) { // https://google.github.io/xls/dslx_reference/#redundant-patterns. absl::flat_hash_set all_match_arms_patterns; for (int64_t arm_count = 0; arm_count < max_arm_count; ++arm_count) { - std::vector match_arm_patterns; + std::vector match_arm_patterns; // Attempt to create at least one pattern. int64_t max_pattern_count = absl::Uniform(absl::IntervalClosed, bit_gen_, 1, 2); for (int64_t pattern_count = 0; pattern_count < max_pattern_count; ++pattern_count) { - XLS_ASSIGN_OR_RETURN(NameDefTree * pattern, + XLS_ASSIGN_OR_RETURN(PatternTree pattern, GenerateMatchArmPattern(ctx, match.type)); // Early exit when a wildcard pattern is created. - if (pattern->is_leaf() && - std::holds_alternative(pattern->leaf())) { + if (IsWildcardLeaf(pattern)) { break; } - std::string pattern_str = pattern->ToString(); + std::string pattern_str = PatternToString(pattern); if (all_match_arms_patterns.contains(pattern_str)) { continue; } @@ -1093,9 +1093,8 @@ absl::StatusOr AstGenerator::GenerateMatch(Context* ctx) { XLS_ASSIGN_OR_RETURN(TypedExpr wc_return, GenerateExprOfType(ctx, match_return_type)); WildcardPattern* wc = module_->Make(fake_span_); - NameDefTree* wc_pattern = module_->Make(fake_span_, wc); match_arms.push_back(module_->Make( - fake_span_, std::vector{wc_pattern}, wc_return.expr)); + fake_span_, std::vector{wc}, wc_return.expr)); last_delaying_op = ComposeDelayingOps(last_delaying_op, wc_return.last_delaying_op); min_stage = std::max(min_stage, wc_return.min_stage); @@ -1214,7 +1213,6 @@ AstGenerator::GeneratePartialProductDeterministicGroup(Context* ctx) { auto* mulp_name_def = module_->Make(fake_span_, mulp_identifier, /*definer=*/nullptr); auto* mulp_name_ref = MakeNameRef(mulp_name_def); - auto* ndt = module_->Make(fake_span_, mulp_name_def); auto mulp_lhs = module_->Make(fake_span_, mulp_name_ref, /*index=*/MakeNumber(0)); auto mulp_rhs = module_->Make(fake_span_, mulp_name_ref, @@ -1224,7 +1222,7 @@ AstGenerator::GeneratePartialProductDeterministicGroup(Context* ctx) { if (is_signed) { // For smul we have to cast the summation to signed. sum = module_->Make(fake_span_, sum, signed_type); } - auto* let = module_->Make(fake_span_, /*name_def_tree=*/ndt, + auto* let = module_->Make(fake_span_, /*pattern=*/mulp_name_def, /*type=*/mulp.type, /*rhs=*/mulp.expr, /*is_const=*/false); auto* body_stmt = module_->Make(let); @@ -1895,24 +1893,22 @@ absl::StatusOr AstGenerator::GenerateCountedFor(Context* ctx) { absl::Uniform(absl::IntervalClosed, bit_gen_, 1, 8), ivar_type); Expr* iterable = MakeRange(zero, trips); NameDef* x_def = MakeNameDef("x"); - NameDefTree* i_ndt = module_->Make(fake_span_, MakeNameDef("i")); - NameDefTree* x_ndt = module_->Make(fake_span_, x_def); - auto* name_def_tree = module_->Make( - fake_span_, std::vector{i_ndt, x_ndt}); + auto* loop_pattern = module_->Make( + fake_span_, std::vector{MakeNameDef("i"), x_def}); XLS_ASSIGN_OR_RETURN(TypedExpr e, ChooseEnvValueNotArray(&ctx->env)); NameRef* body = MakeNameRef(x_def); // Randomly decide to use or not-use the type annotation on the loop. - TupleTypeAnnotation* tree_type = nullptr; + TupleTypeAnnotation* loop_pattern_type = nullptr; if (RandomBool(0.5)) { - tree_type = MakeTupleType({ivar_type, e.type}); + loop_pattern_type = MakeTupleType({ivar_type, e.type}); } Statement* body_stmt = module_->Make(body); auto* block = module_->Make( fake_span_, std::vector{body_stmt}, /*trailing_semi=*/false); - For* for_ = module_->Make(fake_span_, name_def_tree, tree_type, iterable, - block, /*init=*/e.expr); + For* for_ = module_->Make(fake_span_, loop_pattern, loop_pattern_type, + iterable, block, /*init=*/e.expr); return TypedExpr{.expr = for_, .type = e.type, .last_delaying_op = e.last_delaying_op, @@ -2821,12 +2817,12 @@ absl::StatusOr AstGenerator::GenerateBody(int64_t call_depth, // TODO: https://github.com/google/xls/issues/1459 - skip always generating // the full tuple assignment (note, it is currently needed for the // (token, type) case). - statements.push_back(module_->Make(module_->Make( - fake_span_, - /*name_def_tree=*/module_->Make(fake_span_, name_def), - /*type=*/rhs.type, - /*rhs=*/rhs.expr, - /*is_const=*/false))); + statements.push_back( + module_->Make(module_->Make(fake_span_, + /*pattern=*/name_def, + /*type=*/rhs.type, + /*rhs=*/rhs.expr, + /*is_const=*/false))); ctx->env[identifier] = TypedExpr{.expr = name_ref, .type = rhs.type, .last_delaying_op = rhs.last_delaying_op, @@ -2871,8 +2867,7 @@ void AstGenerator::GenerateTupleAssignment( auto* member_name_ref = MakeNameRef(member_name_def); statements.push_back(module_->Make(module_->Make( fake_span_, - /*name_def_tree=*/ - module_->Make(fake_span_, member_name_def), + /*pattern=*/member_name_def, /*type=*/tuple_type->members()[index], /*rhs=*/ module_->Make(fake_span_, name_ref, MakeNumber(index)), @@ -2897,13 +2892,13 @@ void AstGenerator::GenerateTupleAssignment( // TODO: https://github.com/google/xls/issues/1459 - handle tuples of // tuples. - std::vector name_defs; + std::vector pattern_members; bool has_rest_of_tuple = false; for (int64_t index = 0; index < tuple_type->members().size(); ++index) { if (RandomBool(0.1)) { // Replace this name with a wildcard. WildcardPattern* wc = module_->Make(fake_span_); - name_defs.push_back(module_->Make(fake_span_, wc)); + pattern_members.push_back(wc); continue; } @@ -2911,7 +2906,7 @@ void AstGenerator::GenerateTupleAssignment( has_rest_of_tuple = true; // Insert a "rest of tuple", but we might keep this name. RestOfTuple* rest = module_->Make(fake_span_); - name_defs.push_back(module_->Make(fake_span_, rest)); + pattern_members.push_back(rest); // Also, jump forward a random # of elements auto jump_forward = RandomIntWithExpectedValue( /*expected_value=*/(tuple_type->members().size() - index) / 2.0, @@ -2933,13 +2928,12 @@ void AstGenerator::GenerateTupleAssignment( .type = tuple_type->members()[index], .last_delaying_op = rhs.last_delaying_op, .min_stage = rhs.min_stage}; - name_defs.push_back( - module_->Make(fake_span_, member_name_def)); + pattern_members.push_back(member_name_def); } statements.push_back(module_->Make(module_->Make( fake_span_, - /*name_def_tree=*/module_->Make(fake_span_, name_defs), + /*pattern=*/module_->Make(fake_span_, pattern_members), /*type=*/rhs_type, /*rhs=*/rhs.expr, /*is_const=*/false))); diff --git a/xls/fuzzer/ast_generator.h b/xls/fuzzer/ast_generator.h index 68b74f631e..0e04617989 100644 --- a/xls/fuzzer/ast_generator.h +++ b/xls/fuzzer/ast_generator.h @@ -25,7 +25,6 @@ #include #include -#include "gtest/gtest_prod.h" #include "absl/container/btree_map.h" #include "absl/container/flat_hash_map.h" #include "absl/log/check.h" @@ -36,6 +35,7 @@ #include "absl/strings/match.h" #include "absl/strings/str_format.h" #include "absl/types/span.h" +#include "gtest/gtest_prod.h" #include "xls/dslx/frontend/ast.h" #include "xls/dslx/frontend/module.h" #include "xls/dslx/frontend/pos.h" @@ -561,9 +561,8 @@ class AstGenerator { absl::StatusOr GenerateExprOfType(Context* ctx, TypeAnnotation* type); - // Generate a MatchArmPattern with type 'type'. The pattern is represented as - // an xls::dslx::NameDefTree. - absl::StatusOr GenerateMatchArmPattern( + // Generates a match-arm PatternTree with type 'type'. + absl::StatusOr GenerateMatchArmPattern( Context* ctx, const TypeAnnotation* type); // Generate a Match expression. From a4b25333e9284d6b4de235c973588555b18006de Mon Sep 17 00:00:00 2001 From: dank-openai Date: Tue, 21 Jul 2026 12:20:17 -0400 Subject: [PATCH 2/4] dslx: clarify tuple-pattern helper naming Use tuple_pattern and tuple_pattern_type in HandleTuplePatternChild, and update its contract comment to match the surrounding PatternTree vocabulary. This removes the remaining generic tree naming from the helper without changing behavior. Validated with clang-format, git diff --check, and //xls/dslx/type_system_v2:typecheck_module_v2_array_tuple_test on devbox-ubuntu2404. --- .../type_system_v2/populate_table_visitor.cc | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/xls/dslx/type_system_v2/populate_table_visitor.cc b/xls/dslx/type_system_v2/populate_table_visitor.cc index 38c7478c0e..6ca429cf30 100644 --- a/xls/dslx/type_system_v2/populate_table_visitor.cc +++ b/xls/dslx/type_system_v2/populate_table_visitor.cc @@ -762,30 +762,32 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, return absl::OkStatus(); } - // Annotates child `i` of `tree` to be `ElementType(tree_type, i)`, meaning it - // derives its type from the unified type of its parent. The element type - // annotation will refer to it using a right-based index if - // `use_right_based_index_in_type` is true, which implies there is a - // rest-of-tuple node somewhere before child `i`. This function also sets a - // type variable on the child so that its parent-derived type will be unified - // with any other information that traversing it picks up (e.g. if it is a - // literal). + // Annotates child `i` of `tuple_pattern` to be + // `ElementType(tuple_pattern_type, i)`, meaning it derives its type from the + // unified type of its parent. The element type annotation will refer to it + // using a right-based index if `use_right_based_index_in_type` is true, + // which implies there is a rest-of-tuple node somewhere before child `i`. + // This function also sets a type variable on the child so that its + // parent-derived type will be unified with any other information that + // traversing it picks up (e.g. if it is a literal). absl::Status HandleTuplePatternChild( - const TuplePattern* tree, const TypeAnnotation* tree_type, int i, + const TuplePattern* tuple_pattern, + const TypeAnnotation* tuple_pattern_type, int i, bool use_right_based_index_in_type = false) { - const PatternTree& child = tree->members()[i]; + const PatternTree& child = tuple_pattern->members()[i]; const AstNode* actual_child = ToAstNode(child); const TypeAnnotation* element_type = nullptr; if (use_right_based_index_in_type) { // Index from the right uses element count - i. Expr* offset = CreateElementCountOffset( - module_, const_cast(tree_type), - tree->members().size() - i); + module_, const_cast(tuple_pattern_type), + tuple_pattern->members().size() - i); XLS_RETURN_IF_ERROR(DefineAndSetTypeVariable(offset, "offset")); XLS_RETURN_IF_ERROR(table_.SetTypeAnnotation( offset, CreateU32Annotation(module_, GetPatternSpan(child)))); XLS_RETURN_IF_ERROR(offset->Accept(this)); - element_type = module_.Make(tree_type, offset); + element_type = + module_.Make(tuple_pattern_type, offset); } else { // Index from the left just uses a literal. XLS_ASSIGN_OR_RETURN( @@ -793,7 +795,8 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, MakeTypeCheckedNumber( module_, table_, GetPatternSpan(child), i, CreateU32Annotation(module_, GetPatternSpan(child)))); - element_type = module_.Make(tree_type, index); + element_type = + module_.Make(tuple_pattern_type, index); } XLS_RETURN_IF_ERROR( From dd56c3363fa933bfca8304b24443128cd19f1734 Mon Sep 17 00:00:00 2001 From: dank-openai Date: Tue, 21 Jul 2026 12:31:27 -0400 Subject: [PATCH 3/4] dslx: reserve retired NameDefTree proto value Reserve wire value 21 now that NameDefTree is no longer a concrete AST kind. Decode raw value 21 explicitly as an unsupported legacy artifact while TuplePattern remains the new concrete enum value. The focused test now injects the raw legacy value so schema reservation cannot hide the rejection path. Validated with clang-format, git diff --check, and //xls/dslx/type_system:type_info_to_proto_test on devbox-ubuntu2404 with test-result caching disabled. --- xls/dslx/type_system/type_info.proto | 4 ++-- xls/dslx/type_system/type_info_to_proto.cc | 12 ++++++++---- xls/dslx/type_system/type_info_to_proto_test.cc | 8 ++++++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/xls/dslx/type_system/type_info.proto b/xls/dslx/type_system/type_info.proto index 8af524e991..71af3d37c8 100644 --- a/xls/dslx/type_system/type_info.proto +++ b/xls/dslx/type_system/type_info.proto @@ -43,8 +43,8 @@ enum AstNodeKindProto { AST_NODE_KIND_ARRAY = 18; AST_NODE_KIND_STRING = 19; AST_NODE_KIND_STRUCT_INSTANCE = 20; - // Retired legacy wire value; do not reuse. - AST_NODE_KIND_NAME_DEF_TREE = 21; + // Retired legacy NameDefTree wire value; reject it when decoding. + reserved 21; AST_NODE_KIND_SPLAT_STRUCT_INSTANCE = 22; AST_NODE_KIND_INDEX = 23; AST_NODE_KIND_RECV = 24; diff --git a/xls/dslx/type_system/type_info_to_proto.cc b/xls/dslx/type_system/type_info_to_proto.cc index d15415f16a..a2f00f0888 100644 --- a/xls/dslx/type_system/type_info_to_proto.cc +++ b/xls/dslx/type_system/type_info_to_proto.cc @@ -48,6 +48,8 @@ namespace xls::dslx { namespace { +constexpr int kLegacyNameDefTreeAstNodeKindProtoValue = 21; + // Converts the AstNodeKind (C++ enum class) to its protobuf form. AstNodeKindProto ToProto(AstNodeKind kind) { switch (kind) { @@ -668,6 +670,12 @@ absl::StatusOr ToHumanString(const TypeProto& ctp, } absl::StatusOr FromProto(AstNodeKindProto p) { + if (static_cast(p) == kLegacyNameDefTreeAstNodeKindProtoValue) { + return absl::InvalidArgumentError( + "Legacy NameDefTree type-info entries are unsupported; re-typecheck " + "the module"); + } + switch (p) { case AST_NODE_KIND_ATTRIBUTE: return AstNodeKind::kAttribute; @@ -717,10 +725,6 @@ absl::StatusOr FromProto(AstNodeKindProto p) { return AstNodeKind::kStructInstance; case AST_NODE_KIND_STRUCT_MEMBER: return AstNodeKind::kStructMember; - case AST_NODE_KIND_NAME_DEF_TREE: - return absl::InvalidArgumentError( - "Legacy NameDefTree type-info entries are unsupported; re-typecheck " - "the module"); case AST_NODE_KIND_TUPLE_PATTERN: return AstNodeKind::kTuplePattern; case AST_NODE_KIND_SPLAT_STRUCT_INSTANCE: diff --git a/xls/dslx/type_system/type_info_to_proto_test.cc b/xls/dslx/type_system/type_info_to_proto_test.cc index c5c227907b..81a08e827e 100644 --- a/xls/dslx/type_system/type_info_to_proto_test.cc +++ b/xls/dslx/type_system/type_info_to_proto_test.cc @@ -33,6 +33,8 @@ namespace xls::dslx { namespace { +constexpr int kLegacyNameDefTreeAstNodeKindProtoValue = 21; + std::string TestName() { return ::testing::UnitTest::GetInstance()->current_test_info()->name(); } @@ -206,7 +208,8 @@ TEST_F(TypeInfoToProtoWithBothTypecheckVersionsTest, bool found_tuple_pattern = false; for (const AstNodeTypeInfoProto& node : tip.nodes()) { found_tuple_pattern |= node.kind() == AST_NODE_KIND_TUPLE_PATTERN; - EXPECT_NE(node.kind(), AST_NODE_KIND_NAME_DEF_TREE); + EXPECT_NE(static_cast(node.kind()), + kLegacyNameDefTreeAstNodeKindProtoValue); } EXPECT_TRUE(found_tuple_pattern); } @@ -215,7 +218,8 @@ TEST_F(TypeInfoToProtoWithBothTypecheckVersionsTest, RejectsLegacyNameDefTreeAstNodeKind) { ImportData import_data = CreateImportDataForTest(); AstNodeTypeInfoProto legacy; - legacy.set_kind(AST_NODE_KIND_NAME_DEF_TREE); + legacy.set_kind( + static_cast(kLegacyNameDefTreeAstNodeKindProtoValue)); legacy.mutable_type()->mutable_token_type(); EXPECT_THAT( From 440f123204a01c50c1082e79ace8e0b0504fe7e9 Mon Sep 17 00:00:00 2001 From: dank-openai Date: Wed, 5 Aug 2026 12:55:23 -0400 Subject: [PATCH 4/4] dslx: keep PatternTree AST entries sorted --- xls/dslx/frontend/ast_node.h | 2 +- xls/dslx/ir_convert/function_converter.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/xls/dslx/frontend/ast_node.h b/xls/dslx/frontend/ast_node.h index 56cba3cc6b..4b3b41f1e8 100644 --- a/xls/dslx/frontend/ast_node.h +++ b/xls/dslx/frontend/ast_node.h @@ -94,8 +94,8 @@ enum class AstNodeKind : uint8_t { kTestFunction, kTestProc, kTrait, - kTuplePattern, kTupleIndex, + kTuplePattern, kTypeAlias, kTypeAnnotation, kTypeRef, diff --git a/xls/dslx/ir_convert/function_converter.cc b/xls/dslx/ir_convert/function_converter.cc index 09f6039352..6f70ab5acd 100644 --- a/xls/dslx/ir_convert/function_converter.cc +++ b/xls/dslx/ir_convert/function_converter.cc @@ -411,13 +411,13 @@ class FunctionConverterVisitor : public AstNodeVisitor { INVALID(FuzzTestFunction) INVALID(MatchArm) INVALID(NameDef) - INVALID(TuplePattern) INVALID(ParametricBinding) INVALID(ProcAlias) INVALID(RestOfTuple) INVALID(Slice) INVALID(TestFunction) INVALID(TestProc) + INVALID(TuplePattern) INVALID(TypeRef) INVALID(VerbatimNode) INVALID(WidthSlice)