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..4b3b41f1e8 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, @@ -96,6 +95,7 @@ enum class AstNodeKind : uint8_t { kTestProc, kTrait, kTupleIndex, + kTuplePattern, kTypeAlias, kTypeAnnotation, kTypeRef, 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..6f70ab5acd 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,13 +411,13 @@ class FunctionConverterVisitor : public AstNodeVisitor { INVALID(FuzzTestFunction) INVALID(MatchArm) INVALID(NameDef) - INVALID(NameDefTree) INVALID(ParametricBinding) INVALID(ProcAlias) INVALID(RestOfTuple) INVALID(Slice) INVALID(TestFunction) INVALID(TestProc) + INVALID(TuplePattern) INVALID(TypeRef) INVALID(VerbatimNode) INVALID(WidthSlice) @@ -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..71af3d37c8 100644 --- a/xls/dslx/type_system/type_info.proto +++ b/xls/dslx/type_system/type_info.proto @@ -43,7 +43,8 @@ enum AstNodeKindProto { AST_NODE_KIND_ARRAY = 18; AST_NODE_KIND_STRING = 19; AST_NODE_KIND_STRUCT_INSTANCE = 20; - 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; @@ -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..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) { @@ -101,8 +103,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: @@ -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,8 +725,8 @@ 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 AstNodeKind::kNameDefTree; + 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..81a08e827e 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" @@ -33,6 +33,8 @@ namespace xls::dslx { namespace { +constexpr int kLegacyNameDefTreeAstNodeKindProtoValue = 21; + std::string TestName() { return ::testing::UnitTest::GetInstance()->current_test_info()->name(); } @@ -192,5 +194,40 @@ 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(static_cast(node.kind()), + kLegacyNameDefTreeAstNodeKindProtoValue); + } + EXPECT_TRUE(found_tuple_pattern); +} + +TEST_F(TypeInfoToProtoWithBothTypecheckVersionsTest, + RejectsLegacyNameDefTreeAstNodeKind) { + ImportData import_data = CreateImportDataForTest(); + AstNodeTypeInfoProto legacy; + legacy.set_kind( + static_cast(kLegacyNameDefTreeAstNodeKindProtoValue)); + 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..6ca429cf30 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,103 +709,104 @@ 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)); } } 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). - absl::Status HandleNameDefTreeChild( - const NameDefTree* tree, const TypeAnnotation* tree_type, int i, + // 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* tuple_pattern, + const TypeAnnotation* tuple_pattern_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 = 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->nodes().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_, child->span()))); + 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( Number * index, - MakeTypeCheckedNumber(module_, table_, child->span(), i, - CreateU32Annotation(module_, child->span()))); - element_type = module_.Make(tree_type, index); + MakeTypeCheckedNumber( + module_, table_, GetPatternSpan(child), i, + CreateU32Annotation(module_, GetPatternSpan(child)))); + element_type = + module_.Make(tuple_pattern_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 +857,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 +885,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 +930,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 +1896,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.