Skip to content

dslx: reject duplicate match alternatives - #4731

Open
dank-openai wants to merge 1 commit into
google:mainfrom
xlsynth:dank/wip/upstream-dslx-duplicate-match-alternatives-0806
Open

dslx: reject duplicate match alternatives#4731
dank-openai wants to merge 1 commit into
google:mainfrom
xlsynth:dank/wip/upstream-dslx-duplicate-match-alternatives-0806

Conversation

@dank-openai

Copy link
Copy Markdown
Collaborator

Summary

  • Reject duplicate DSLX match patterns inside grouped | alternatives.
  • Point to both the original match pattern and its unreachable duplicate.
  • Preserve valid grouped alternatives and existing duplicate-arm checks.

Problem

DSLX currently accepts this unreachable second E::A pattern:

match value {
  E::A => u32:0,
  E::B | E::A => u32:1,
  E::C => u32:2,
}

The compiler compares entire match arms, so it misses a repeated pattern nested
inside another arm's alternatives. Track the individual alternatives as well,
and report both occurrences when one repeats.

Programs containing exact duplicate alternatives now intentionally fail
compilation. Valid grouped alternatives continue to compile.

The separate pre-existing enum-alias spelling case remains documented by a
disabled regression and is not fixed by this change.

Tests

The equivalent merged producer change passed:

  • //xls/dslx/type_system_v2:typecheck_module_v2_control_flow_test
  • //xls/dslx/exhaustiveness:exhaustiveness_match_test
  • //xls/dslx/type_system:typecheck_module_test

The added diagnostic fixture is owned by
//xls/dslx/tests/errors:error_modules_test.

Current upstream main cannot start an ordinary Bazel build because its
rules_hdl patch targets a missing file; the same failure is already present
in upstream main CI.
No unrelated dependency or build-configuration changes are included here.

* dslx: reject duplicate match alternatives with both source locations

## Summary

- Reject duplicate match patterns even when an occurrence is embedded in a `|`-separated alternative.
- Highlight both the original and duplicate source locations in the compiler error.
- Preserve valid grouped alternatives and existing duplicate-arm checks.
- Preserve a disabled regression for the separate pre-existing case where an enum type alias gives the same variant two spellings.

## Problem Solved

DSLX compared complete match arms when checking for duplicates. Consequently, an enum variant could appear in separate arms and the match would still compile:

```dslx
match value {
  E::A => u32:0,
  E::B | E::A => u32:1,
  E::C => u32:2,
}
```

The second `E::A` can never match, hiding unreachable code and copy-and-paste mistakes. This change rejects the program with a `TypeInferenceError` and reports both occurrences.

## Implementation

Track individual top-level match alternatives alongside the existing whole-arm duplicate check. Retain the first occurrence so the error can name its location and attach both source spans to the diagnostic.

Programs that previously compiled with duplicate exact match alternatives now intentionally fail compilation. Valid grouped alternatives and existing range or tuple overlap behavior remain unchanged.

Enum type aliases such as `type Alias = E;` can still spell the same member as `E::A` and `Alias::A`. That pre-existing semantic-equivalence case is intentionally not fixed here; a repository-conventional `DISABLED_` regression records the failing behavior for future work.

## Testing

- `bazel test //xls/dslx/type_system_v2:typecheck_module_v2_control_flow_test //xls/dslx/exhaustiveness:exhaustiveness_match_test //xls/dslx/type_system:typecheck_module_test`
- Regression coverage verifies both source spans, rejects the reported enum pattern, and accepts unique grouped alternatives.
- Direct compiler checks cover the checked-in diagnostic fixture, existing whole-arm duplicates, and non-exhaustive enum matches.
- Forcing `DISABLED_MatchEnumVariantDuplicatedThroughTypeAlias` to run with `--gtest_also_run_disabled_tests` fails as expected; normal execution passes 100 control-flow tests and reports one disabled test.

* dslx: explain unresolved duplicate enum-alias match cases

E::A and Alias::A denote the same enum member after type Alias = E,
but match validation compares source spellings and incorrectly accepts
an unreachable duplicate arm.

Record beside the disabled regression that fixing this requires
resolving enum-member identity before comparing match patterns.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 6, 2026
* dslx: reject duplicate match alternatives with both source locations

## Summary

- Reject duplicate match patterns even when an occurrence is embedded in a `|`-separated alternative.
- Highlight both the original and duplicate source locations in the compiler error.
- Preserve valid grouped alternatives and existing duplicate-arm checks.
- Preserve a disabled regression for the separate pre-existing case where an enum type alias gives the same variant two spellings.

## Problem Solved

DSLX compared complete match arms when checking for duplicates. Consequently, an enum variant could appear in separate arms and the match would still compile:

```dslx
match value {
  E::A => u32:0,
  E::B | E::A => u32:1,
  E::C => u32:2,
}
```

The second `E::A` can never match, hiding unreachable code and copy-and-paste mistakes. This change rejects the program with a `TypeInferenceError` and reports both occurrences.

## Implementation

Track individual top-level match alternatives alongside the existing whole-arm duplicate check. Retain the first occurrence so the error can name its location and attach both source spans to the diagnostic.

Programs that previously compiled with duplicate exact match alternatives now intentionally fail compilation. Valid grouped alternatives and existing range or tuple overlap behavior remain unchanged.

Enum type aliases such as `type Alias = E;` can still spell the same member as `E::A` and `Alias::A`. That pre-existing semantic-equivalence case is intentionally not fixed here; a repository-conventional `DISABLED_` regression records the failing behavior for future work.

## Testing

- `bazel test //xls/dslx/type_system_v2:typecheck_module_v2_control_flow_test //xls/dslx/exhaustiveness:exhaustiveness_match_test //xls/dslx/type_system:typecheck_module_test`
- Regression coverage verifies both source spans, rejects the reported enum pattern, and accepts unique grouped alternatives.
- Direct compiler checks cover the checked-in diagnostic fixture, existing whole-arm duplicates, and non-exhaustive enum matches.
- Forcing `DISABLED_MatchEnumVariantDuplicatedThroughTypeAlias` to run with `--gtest_also_run_disabled_tests` fails as expected; normal execution passes 100 control-flow tests and reports one disabled test.

* dslx: explain unresolved duplicate enum-alias match cases

E::A and Alias::A denote the same enum member after type Alias = E,
but match validation compares source spellings and incorrectly accepts
an unreachable duplicate arm.

Record beside the disabled regression that fixing this requires
resolving enum-member identity before comparing match patterns.
@dank-openai
dank-openai force-pushed the dank/wip/upstream-dslx-duplicate-match-alternatives-0806 branch from b35f5ad to dd96cea Compare August 6, 2026 21:26
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 6, 2026
Store first-occurrence source spans directly instead of PatternTree pointers.
This keeps duplicate-pattern and enum-alias diagnostics synchronized with
Google XLS PRs google#4731 and google#4732 without changing their reported locations.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 6, 2026
* dslx: reject duplicate match alternatives with both source locations

## Summary

- Reject duplicate match patterns even when an occurrence is embedded in a `|`-separated alternative.
- Highlight both the original and duplicate source locations in the compiler error.
- Preserve valid grouped alternatives and existing duplicate-arm checks.
- Preserve a disabled regression for the separate pre-existing case where an enum type alias gives the same variant two spellings.

## Problem Solved

DSLX compared complete match arms when checking for duplicates. Consequently, an enum variant could appear in separate arms and the match would still compile:

```dslx
match value {
  E::A => u32:0,
  E::B | E::A => u32:1,
  E::C => u32:2,
}
```

The second `E::A` can never match, hiding unreachable code and copy-and-paste mistakes. This change rejects the program with a `TypeInferenceError` and reports both occurrences.

## Implementation

Track individual top-level match alternatives alongside the existing whole-arm duplicate check. Retain the first occurrence so the error can name its location and attach both source spans to the diagnostic.

Programs that previously compiled with duplicate exact match alternatives now intentionally fail compilation. Valid grouped alternatives and existing range or tuple overlap behavior remain unchanged.

Enum type aliases such as `type Alias = E;` can still spell the same member as `E::A` and `Alias::A`. That pre-existing semantic-equivalence case is intentionally not fixed here; a repository-conventional `DISABLED_` regression records the failing behavior for future work.

## Testing

- `bazel test //xls/dslx/type_system_v2:typecheck_module_v2_control_flow_test //xls/dslx/exhaustiveness:exhaustiveness_match_test //xls/dslx/type_system:typecheck_module_test`
- Regression coverage verifies both source spans, rejects the reported enum pattern, and accepts unique grouped alternatives.
- Direct compiler checks cover the checked-in diagnostic fixture, existing whole-arm duplicates, and non-exhaustive enum matches.
- Forcing `DISABLED_MatchEnumVariantDuplicatedThroughTypeAlias` to run with `--gtest_also_run_disabled_tests` fails as expected; normal execution passes 100 control-flow tests and reports one disabled test.

* dslx: explain unresolved duplicate enum-alias match cases

E::A and Alias::A denote the same enum member after type Alias = E,
but match validation compares source spellings and incorrectly accepts
an unreachable duplicate arm.

Record beside the disabled regression that fixing this requires
resolving enum-member identity before comparing match patterns.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 6, 2026
Store first-occurrence source spans directly instead of PatternTree pointers.
This keeps duplicate-pattern and enum-alias diagnostics synchronized with
Google XLS PRs google#4731 and google#4732 without changing their reported locations.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 7, 2026
* dslx: reject duplicate match alternatives with both source locations

## Summary

- Reject duplicate match patterns even when an occurrence is embedded in a `|`-separated alternative.
- Highlight both the original and duplicate source locations in the compiler error.
- Preserve valid grouped alternatives and existing duplicate-arm checks.
- Preserve a disabled regression for the separate pre-existing case where an enum type alias gives the same variant two spellings.

## Problem Solved

DSLX compared complete match arms when checking for duplicates. Consequently, an enum variant could appear in separate arms and the match would still compile:

```dslx
match value {
  E::A => u32:0,
  E::B | E::A => u32:1,
  E::C => u32:2,
}
```

The second `E::A` can never match, hiding unreachable code and copy-and-paste mistakes. This change rejects the program with a `TypeInferenceError` and reports both occurrences.

## Implementation

Track individual top-level match alternatives alongside the existing whole-arm duplicate check. Retain the first occurrence so the error can name its location and attach both source spans to the diagnostic.

Programs that previously compiled with duplicate exact match alternatives now intentionally fail compilation. Valid grouped alternatives and existing range or tuple overlap behavior remain unchanged.

Enum type aliases such as `type Alias = E;` can still spell the same member as `E::A` and `Alias::A`. That pre-existing semantic-equivalence case is intentionally not fixed here; a repository-conventional `DISABLED_` regression records the failing behavior for future work.

## Testing

- `bazel test //xls/dslx/type_system_v2:typecheck_module_v2_control_flow_test //xls/dslx/exhaustiveness:exhaustiveness_match_test //xls/dslx/type_system:typecheck_module_test`
- Regression coverage verifies both source spans, rejects the reported enum pattern, and accepts unique grouped alternatives.
- Direct compiler checks cover the checked-in diagnostic fixture, existing whole-arm duplicates, and non-exhaustive enum matches.
- Forcing `DISABLED_MatchEnumVariantDuplicatedThroughTypeAlias` to run with `--gtest_also_run_disabled_tests` fails as expected; normal execution passes 100 control-flow tests and reports one disabled test.

* dslx: explain unresolved duplicate enum-alias match cases

E::A and Alias::A denote the same enum member after type Alias = E,
but match validation compares source spellings and incorrectly accepts
an unreachable duplicate arm.

Record beside the disabled regression that fixing this requires
resolving enum-member identity before comparing match patterns.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 7, 2026
Store first-occurrence source spans directly instead of PatternTree pointers.
This keeps duplicate-pattern and enum-alias diagnostics synchronized with
Google XLS PRs google#4731 and google#4732 without changing their reported locations.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 7, 2026
* dslx: reject duplicate match alternatives with both source locations

## Summary

- Reject duplicate match patterns even when an occurrence is embedded in a `|`-separated alternative.
- Highlight both the original and duplicate source locations in the compiler error.
- Preserve valid grouped alternatives and existing duplicate-arm checks.
- Preserve a disabled regression for the separate pre-existing case where an enum type alias gives the same variant two spellings.

## Problem Solved

DSLX compared complete match arms when checking for duplicates. Consequently, an enum variant could appear in separate arms and the match would still compile:

```dslx
match value {
  E::A => u32:0,
  E::B | E::A => u32:1,
  E::C => u32:2,
}
```

The second `E::A` can never match, hiding unreachable code and copy-and-paste mistakes. This change rejects the program with a `TypeInferenceError` and reports both occurrences.

## Implementation

Track individual top-level match alternatives alongside the existing whole-arm duplicate check. Retain the first occurrence so the error can name its location and attach both source spans to the diagnostic.

Programs that previously compiled with duplicate exact match alternatives now intentionally fail compilation. Valid grouped alternatives and existing range or tuple overlap behavior remain unchanged.

Enum type aliases such as `type Alias = E;` can still spell the same member as `E::A` and `Alias::A`. That pre-existing semantic-equivalence case is intentionally not fixed here; a repository-conventional `DISABLED_` regression records the failing behavior for future work.

## Testing

- `bazel test //xls/dslx/type_system_v2:typecheck_module_v2_control_flow_test //xls/dslx/exhaustiveness:exhaustiveness_match_test //xls/dslx/type_system:typecheck_module_test`
- Regression coverage verifies both source spans, rejects the reported enum pattern, and accepts unique grouped alternatives.
- Direct compiler checks cover the checked-in diagnostic fixture, existing whole-arm duplicates, and non-exhaustive enum matches.
- Forcing `DISABLED_MatchEnumVariantDuplicatedThroughTypeAlias` to run with `--gtest_also_run_disabled_tests` fails as expected; normal execution passes 100 control-flow tests and reports one disabled test.

* dslx: explain unresolved duplicate enum-alias match cases

E::A and Alias::A denote the same enum member after type Alias = E,
but match validation compares source spellings and incorrectly accepts
an unreachable duplicate arm.

Record beside the disabled regression that fixing this requires
resolving enum-member identity before comparing match patterns.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 7, 2026
Store first-occurrence source spans directly instead of PatternTree pointers.
This keeps duplicate-pattern and enum-alias diagnostics synchronized with
Google XLS PRs google#4731 and google#4732 without changing their reported locations.
@dank-openai
dank-openai removed the request for review from richmckeever August 7, 2026 15:45
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 7, 2026
* dslx: reject duplicate match alternatives with both source locations

## Summary

- Reject duplicate match patterns even when an occurrence is embedded in a `|`-separated alternative.
- Highlight both the original and duplicate source locations in the compiler error.
- Preserve valid grouped alternatives and existing duplicate-arm checks.
- Preserve a disabled regression for the separate pre-existing case where an enum type alias gives the same variant two spellings.

## Problem Solved

DSLX compared complete match arms when checking for duplicates. Consequently, an enum variant could appear in separate arms and the match would still compile:

```dslx
match value {
  E::A => u32:0,
  E::B | E::A => u32:1,
  E::C => u32:2,
}
```

The second `E::A` can never match, hiding unreachable code and copy-and-paste mistakes. This change rejects the program with a `TypeInferenceError` and reports both occurrences.

## Implementation

Track individual top-level match alternatives alongside the existing whole-arm duplicate check. Retain the first occurrence so the error can name its location and attach both source spans to the diagnostic.

Programs that previously compiled with duplicate exact match alternatives now intentionally fail compilation. Valid grouped alternatives and existing range or tuple overlap behavior remain unchanged.

Enum type aliases such as `type Alias = E;` can still spell the same member as `E::A` and `Alias::A`. That pre-existing semantic-equivalence case is intentionally not fixed here; a repository-conventional `DISABLED_` regression records the failing behavior for future work.

## Testing

- `bazel test //xls/dslx/type_system_v2:typecheck_module_v2_control_flow_test //xls/dslx/exhaustiveness:exhaustiveness_match_test //xls/dslx/type_system:typecheck_module_test`
- Regression coverage verifies both source spans, rejects the reported enum pattern, and accepts unique grouped alternatives.
- Direct compiler checks cover the checked-in diagnostic fixture, existing whole-arm duplicates, and non-exhaustive enum matches.
- Forcing `DISABLED_MatchEnumVariantDuplicatedThroughTypeAlias` to run with `--gtest_also_run_disabled_tests` fails as expected; normal execution passes 100 control-flow tests and reports one disabled test.

* dslx: explain unresolved duplicate enum-alias match cases

E::A and Alias::A denote the same enum member after type Alias = E,
but match validation compares source spellings and incorrectly accepts
an unreachable duplicate arm.

Record beside the disabled regression that fixing this requires
resolving enum-member identity before comparing match patterns.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 7, 2026
Store first-occurrence source spans directly instead of PatternTree pointers.
This keeps duplicate-pattern and enum-alias diagnostics synchronized with
Google XLS PRs google#4731 and google#4732 without changing their reported locations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant