diff --git a/xls/dslx/tests/errors/duplicate_enum_match_arm.x b/xls/dslx/tests/errors/duplicate_enum_match_arm.x new file mode 100644 index 0000000000..ccc4cf7e40 --- /dev/null +++ b/xls/dslx/tests/errors/duplicate_enum_match_arm.x @@ -0,0 +1,23 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +enum Example: u2 { A = 0, B = 1, C = 2 } + +fn main(value: Example) -> u32 { + match value { + Example::A => u32:0, + Example::B | Example::A => u32:1, + Example::C => u32:2, + } +} diff --git a/xls/dslx/tests/errors/error_modules_test.py b/xls/dslx/tests/errors/error_modules_test.py index 730f82b915..98e643e730 100644 --- a/xls/dslx/tests/errors/error_modules_test.py +++ b/xls/dslx/tests/errors/error_modules_test.py @@ -538,6 +538,18 @@ def test_duplicate_match_arm(self): self.assertIn('duplicate_match_arm.x:22:5-22:8', stderr) self.assertIn('Exact-duplicate pattern match detected `FOO`', stderr) + def test_duplicate_enum_match_arm(self): + stderr = self._run( + 'xls/dslx/tests/errors/duplicate_enum_match_arm.x', + ) + self.assertIn('duplicate_enum_match_arm.x:19:5-19:15', stderr) + self.assertIn('duplicate_enum_match_arm.x:20:18-20:28', stderr) + self.assertIn('Exact-duplicate pattern match detected `Example::A`', stderr) + self.assertIn( + 'previously @ xls/dslx/tests/errors/duplicate_enum_match_arm.x:19:5-19:15', + stderr, + ) + def test_trace_fmt_empty_err(self): stderr = self._run( 'xls/dslx/tests/errors/trace_fmt_empty.x', diff --git a/xls/dslx/type_system_v2/BUILD b/xls/dslx/type_system_v2/BUILD index 99e469768d..5d5cf1ceae 100644 --- a/xls/dslx/type_system_v2/BUILD +++ b/xls/dslx/type_system_v2/BUILD @@ -535,6 +535,7 @@ cc_library( "//xls/dslx:errors", "//xls/dslx:import_data", "//xls/dslx:import_routines", + "//xls/dslx:status_payload_utils", "//xls/dslx/frontend:ast", "//xls/dslx/frontend:ast_cloner", "//xls/dslx/frontend:ast_node", diff --git a/xls/dslx/type_system_v2/import_utils.cc b/xls/dslx/type_system_v2/import_utils.cc index 067e716cfe..7e8f2d75e8 100644 --- a/xls/dslx/type_system_v2/import_utils.cc +++ b/xls/dslx/type_system_v2/import_utils.cc @@ -346,6 +346,24 @@ absl::StatusOr> GetEnumDef( return unwrapper.GetEnumDef(); } +absl::StatusOr> ResolveEnumMember( + const ColonRef* colon_ref, const ImportData& import_data) { + TypeRefUnwrapper unwrapper(import_data); + XLS_RETURN_IF_ERROR(ToAstNode(colon_ref->subject())->Accept(&unwrapper)); + + std::optional enum_member; + if (std::optional enum_def = unwrapper.GetEnumDef(); + enum_def.has_value()) { + for (const EnumMember& member : (*enum_def)->values()) { + if (member.name_def->identifier() == colon_ref->attr()) { + enum_member = member.name_def; + break; + } + } + } + return enum_member; +} + bool IsImport(const ColonRef* colon_ref) { if (colon_ref->ResolveImportSubject().has_value()) { return true; diff --git a/xls/dslx/type_system_v2/import_utils.h b/xls/dslx/type_system_v2/import_utils.h index 61575cb93e..55fe9083c9 100644 --- a/xls/dslx/type_system_v2/import_utils.h +++ b/xls/dslx/type_system_v2/import_utils.h @@ -105,6 +105,11 @@ absl::StatusOr> GetImportedModuleInfo( absl::StatusOr> GetEnumDef( const TypeAnnotation* annotation, const ImportData& import_data); +// Resolves an enum member through type aliases. Returns `nullopt` if the +// reference does not identify an enum or the enum has no matching member. +absl::StatusOr> ResolveEnumMember( + const ColonRef* colon_ref, const ImportData& import_data); + // Returns whether `f` is a `next` function in an impl-style proc. absl::StatusOr IsProcDefNextFunction(const Function* f, const ImportData& import_data); diff --git a/xls/dslx/type_system_v2/populate_table_visitor.cc b/xls/dslx/type_system_v2/populate_table_visitor.cc index 6ca429cf30..f7a4ee7856 100644 --- a/xls/dslx/type_system_v2/populate_table_visitor.cc +++ b/xls/dslx/type_system_v2/populate_table_visitor.cc @@ -55,6 +55,7 @@ #include "xls/dslx/frontend/pos.h" #include "xls/dslx/import_data.h" #include "xls/dslx/import_routines.h" +#include "xls/dslx/status_payload_utils.h" #include "xls/dslx/type_system/deduce_utils.h" #include "xls/dslx/type_system_v2/import_utils.h" #include "xls/dslx/type_system_v2/inference_table.h" @@ -594,19 +595,29 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, file_table_); } + auto duplicate_pattern_error = [this](const Span& original_span, + const Span& duplicate_span, + std::string_view pattern) { + absl::Status status = absl::InvalidArgumentError(absl::StrFormat( + "TypeInferenceError: Exact-duplicate pattern match detected `%s`; " + "only the first could possibly match; previously @ %s", + pattern, original_span.ToString(file_table_))); + AddSpanToStatusPayload(status, duplicate_span, import_data_.file_table()); + AddSpanToStatusPayload(status, original_span, import_data_.file_table()); + return status; + }; + std::vector type_annotation_members; - absl::flat_hash_set seen_patterns; + absl::flat_hash_map seen_arms; + absl::flat_hash_map seen_patterns; + absl::flat_hash_map seen_enum_members; for (MatchArm* arm : node->arms()) { // Identify syntactically identical match arms. std::string patterns_string = PatternsToString(arm); - if (auto [it, inserted] = seen_patterns.insert(patterns_string); + if (auto [it, inserted] = seen_arms.try_emplace(patterns_string, arm); !inserted) { - return TypeInferenceErrorStatus( - arm->GetPatternSpan(), nullptr, - absl::StrFormat("Exact-duplicate pattern match detected `%s`; only " - "the first could possibly match", - patterns_string), - file_table_); + return duplicate_pattern_error(it->second->GetPatternSpan(), + arm->GetPatternSpan(), patterns_string); } if (node->IsConst()) { @@ -624,6 +635,28 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, } for (const PatternTree& pattern : arm->patterns()) { + if (auto [it, inserted] = seen_patterns.try_emplace( + PatternToString(pattern), GetPatternSpan(pattern)); + !inserted) { + return duplicate_pattern_error(it->second, GetPatternSpan(pattern), + it->first); + } + + if (std::holds_alternative(pattern)) { + const ColonRef* colon_ref = std::get(pattern); + XLS_ASSIGN_OR_RETURN(std::optional enum_member, + ResolveEnumMember(colon_ref, import_data_)); + if (enum_member.has_value()) { + if (auto [it, inserted] = seen_enum_members.try_emplace( + *enum_member, GetPatternSpan(pattern)); + !inserted) { + return duplicate_pattern_error(it->second, + GetPatternSpan(pattern), + PatternToString(pattern)); + } + } + } + XLS_RETURN_IF_ERROR( table_.SetTypeVariable(ToAstNode(pattern), matched_var)); } diff --git a/xls/dslx/type_system_v2/typecheck_module_v2_control_flow_test.cc b/xls/dslx/type_system_v2/typecheck_module_v2_control_flow_test.cc index 3f61ebae3a..a43c3ab6fc 100644 --- a/xls/dslx/type_system_v2/typecheck_module_v2_control_flow_test.cc +++ b/xls/dslx/type_system_v2/typecheck_module_v2_control_flow_test.cc @@ -880,6 +880,144 @@ const Z = match X { HasSubstr("Exact-duplicate pattern match detected `u32:1`"))); } +TEST(TypecheckV2Test, MatchEnumVariantDuplicatedAcrossAlternativeArms) { + constexpr std::string_view kProgram = R"( +enum E: u2 { + A = 0, + B = 1, + C = 2, +} + +fn f(value: E) -> u32 { + match value { + E::A => u32:0, + E::B | E::A => u32:1, + E::C => u32:2, + } +} +)"; + + EXPECT_THAT( + kProgram, + TypecheckFailsWithPayload( + AllOf(HasSubstr("Exact-duplicate pattern match detected `E::A`"), + HasSubstr("previously @ fake.x:12:5-12:9")), + AllOf(HasSpan(11, 4, 11, 8), HasSpan(12, 11, 12, 15)))); +} + +// Type aliases do not create distinct enum members, even when their match +// patterns have different source spellings. +TEST(TypecheckV2Test, MatchEnumVariantDuplicatedThroughTypeAlias) { + EXPECT_THAT( + R"( +enum E: u2 { A = 0, B = 1 } +type Alias = E; + +fn f(value: E) -> u32 { + match value { + E::A => u32:0, + Alias::A => u32:1, + E::B => u32:2, + } +} +)", + TypecheckFailsWithPayload( + AllOf(HasSubstr("Exact-duplicate pattern match detected `Alias::A`"), + HasSubstr("previously @ fake.x:9:5-9:9")), + AllOf(HasSpan(8, 4, 8, 8), HasSpan(9, 4, 9, 12)))); +} + +TEST(TypecheckV2Test, MatchEnumVariantDuplicatedThroughChainedTypeAliases) { + EXPECT_THAT( + R"( +enum E: u2 { A = 0, B = 1 } +type First = E; +type Second = First; + +fn f(value: E) -> u32 { + match value { + First::A => u32:0, + Second::A => u32:1, + E::B => u32:2, + } +} +)", + TypecheckFails( + HasSubstr("Exact-duplicate pattern match detected `Second::A`"))); +} + +TEST(TypecheckV2Test, + MatchEnumVariantDuplicatedThroughTypeAliasInGroupedAlternatives) { + EXPECT_THAT( + R"( +enum E: u2 { A = 0, B = 1, C = 2 } +type Alias = E; + +fn f(value: E) -> u32 { + match value { + E::A => u32:0, + E::B | Alias::A => u32:1, + E::C => u32:2, + } +} +)", + TypecheckFails( + HasSubstr("Exact-duplicate pattern match detected `Alias::A`"))); +} + +TEST(TypecheckV2Test, MatchImportedEnumVariantDuplicatedThroughTypeAlias) { + constexpr std::string_view kImported = R"( +pub enum E: u2 { A = 0, B = 1 } +pub type ImportedAlias = E; +)"; + constexpr std::string_view kProgram = R"( +import imported; +type Alias = imported::ImportedAlias; + +fn f(value: Alias) -> u32 { + match value { + imported::E::A => u32:0, + Alias::A => u32:1, + imported::E::B => u32:2, + } +} +)"; + + ImportData import_data = CreateImportDataForTest(); + XLS_EXPECT_OK(TypecheckV2(kImported, "imported", &import_data)); + EXPECT_THAT( + TypecheckV2(kProgram, "main", &import_data), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Exact-duplicate pattern match detected `Alias::A`"))); +} + +TEST(TypecheckV2Test, MatchDistinctEnumVariantsThroughTypeAliasRemainValid) { + XLS_EXPECT_OK(TypecheckV2(R"( +enum E: u2 { A = 0, B = 1, C = 2 } +type Alias = E; + +fn f(value: E) -> u32 { + match value { + E::A => u32:0, + Alias::B | Alias::C => u32:1, + } +} +)")); +} + +TEST(TypecheckV2Test, MatchEnumVariantAlternativesRemainValid) { + XLS_EXPECT_OK(TypecheckV2(R"( +enum E: u2 { A = 0, B = 1, C = 2 } + +fn f(value: E) -> u32 { + match value { + E::A => u32:0, + E::B | E::C => u32:1, + } +} +)")); +} + TEST(TypecheckV2Test, MatchNonExhaustive) { EXPECT_THAT(R"( fn f(x: u1) -> u32 {