Skip to content

dslx: reject duplicate enum match alternatives through type aliases - #4732

Open
dank-openai wants to merge 2 commits into
google:mainfrom
xlsynth:dank/wip/upstream-dslx-enum-alias-duplicate-match-0806
Open

dslx: reject duplicate enum match alternatives through type aliases#4732
dank-openai wants to merge 2 commits into
google:mainfrom
xlsynth:dank/wip/upstream-dslx-enum-alias-duplicate-match-0806

Conversation

@dank-openai

Copy link
Copy Markdown
Collaborator

Summary

  • Reject duplicate enum match alternatives even when type aliases give the same
    member different source spellings.
  • Report both the original and duplicate source locations.
  • Cover direct aliases, alias chains, grouped alternatives, imported aliases,
    and valid distinct members.

Depends on #4731. This branch adds one focused, four-file commit on top of that
open prerequisite; GitHub's main-based diff will also include #4731 until it
merges. The recently merged pattern-tree refactor in #4614 currently conflicts
with #4731 in populate_table_visitor.cc; that prerequisite must be reconciled
with current main before this dependent change can land.

Problem

An enum type alias does not create new enum members, so the second arm below can
never match:

type Alias = E;

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

The existing duplicate check compares source spellings, so E::A and
Alias::A incorrectly appear distinct. The compiler now rejects the program
with the existing two-location diagnostic:

fake.x:10:5-10:13
0009:     E::A => u32:0,
0010:     Alias::A => u32:1,
~~~~~~~~~~^------^ TypeInferenceError: Exact-duplicate pattern match detected `Alias::A`; only the first could possibly match; previously @ fake.x:9:5-9:9
fake.x:9:5-9:9
0009:     E::A => u32:0,
~~~~~~~~~~^--^

Implementation

Resolve enum alternatives through the existing type-alias and import resolver,
then compare their underlying enum-member declarations. Keep the existing
whole-arm and source-spelling checks unchanged.

Distinct declared members remain distinct even when they share an underlying
numeric value. Existing grouped alternatives, unknown-member diagnostics, and
other match-pattern semantics are preserved.

Tests

  • The direct-alias regression checks both exact source spans.
  • Chained aliases, grouped alternatives, imported aliases, and valid distinct
    members each have focused regression coverage.
  • The complete upstream
    //xls/dslx/type_system_v2:typecheck_module_v2_control_flow_test target
    passes, including all five focused alias cases, with validation-only
    dependency and macOS deployment-target overrides.
  • The equivalent merged producer change passed the complete DSLX control-flow,
    exhaustiveness, and existing typechecker suites.
  • 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.

@dank-openai
dank-openai requested review from meheff and richmckeever and removed request for meheff August 6, 2026 19:54
* 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.
Enum type aliases can give the same match alternative different source
spellings, allowing unreachable duplicate arms. Resolve enum references
through the existing type and import resolver, compare their declared member
identities, and preserve the diagnostic spans for both occurrences.

Enable the direct-alias regression and cover chained aliases, grouped
alternatives, imported aliases, and valid matches with distinct members.
dank-openai added a commit to xlsynth/xlsynth that referenced this pull request Aug 6, 2026
…hrough type aliases (#31)

## Summary

- Reject duplicate enum match alternatives even when type aliases, alias
  chains, or imported aliases give the same member different source spellings.
- Preserve the existing compiler diagnostic with both original and duplicate
  source locations.
- Enable the previously disabled alias regression and cover grouped, chained,
  imported, and valid distinct-member cases.

## Problem Solved

An enum type alias does not create a new enum member, so both alternatives in
this match select the same enum member:

```dslx
type Alias = E;

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

The previous duplicate checker compared source spelling and accepted the
unreachable second arm because `E::A` and `Alias::A` are different strings.

## Implementation

Resolve enum alternatives through the existing type-alias and import resolver,
then compare their underlying enum-member declarations. Keep the existing
whole-arm and exact-spelling checks unchanged, and reuse their two-location
compiler diagnostic.

Distinct members remain distinct even when they share an underlying numeric
value. Existing tuple, range, grouped-alternative, and unknown-member behavior
is preserved.

## Testing

- The previously disabled direct-alias regression and new chained, grouped, and
  imported-alias regressions fail before the fix and pass afterward.
- Valid grouped alternatives and distinct aliased enum members remain accepted.
- Full control-flow, exhaustiveness, and legacy typechecker suites pass.
- The enum suite passes excluding one inherited, unrelated semantic-sum
  constructor diagnostic mismatch already present on the parent branch.

pr:enum-alias-duplicate-match
@dank-openai
dank-openai force-pushed the dank/wip/upstream-dslx-enum-alias-duplicate-match-0806 branch from 5151eb1 to d745795 Compare August 6, 2026 21:27
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
…hrough type aliases (#31)

## Summary

- Reject duplicate enum match alternatives even when type aliases, alias
  chains, or imported aliases give the same member different source spellings.
- Preserve the existing compiler diagnostic with both original and duplicate
  source locations.
- Enable the previously disabled alias regression and cover grouped, chained,
  imported, and valid distinct-member cases.

## Problem Solved

An enum type alias does not create a new enum member, so both alternatives in
this match select the same enum member:

```dslx
type Alias = E;

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

The previous duplicate checker compared source spelling and accepted the
unreachable second arm because `E::A` and `Alias::A` are different strings.

## Implementation

Resolve enum alternatives through the existing type-alias and import resolver,
then compare their underlying enum-member declarations. Keep the existing
whole-arm and exact-spelling checks unchanged, and reuse their two-location
compiler diagnostic.

Distinct members remain distinct even when they share an underlying numeric
value. Existing tuple, range, grouped-alternative, and unknown-member behavior
is preserved.

## Testing

- The previously disabled direct-alias regression and new chained, grouped, and
  imported-alias regressions fail before the fix and pass afterward.
- Valid grouped alternatives and distinct aliased enum members remain accepted.
- Full control-flow, exhaustiveness, and legacy typechecker suites pass.
- The enum suite passes excluding one inherited, unrelated semantic-sum
  constructor diagnostic mismatch already present on the parent branch.

pr:enum-alias-duplicate-match
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
…hrough type aliases (#31)

## Summary

- Reject duplicate enum match alternatives even when type aliases, alias
  chains, or imported aliases give the same member different source spellings.
- Preserve the existing compiler diagnostic with both original and duplicate
  source locations.
- Enable the previously disabled alias regression and cover grouped, chained,
  imported, and valid distinct-member cases.

## Problem Solved

An enum type alias does not create a new enum member, so both alternatives in
this match select the same enum member:

```dslx
type Alias = E;

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

The previous duplicate checker compared source spelling and accepted the
unreachable second arm because `E::A` and `Alias::A` are different strings.

## Implementation

Resolve enum alternatives through the existing type-alias and import resolver,
then compare their underlying enum-member declarations. Keep the existing
whole-arm and exact-spelling checks unchanged, and reuse their two-location
compiler diagnostic.

Distinct members remain distinct even when they share an underlying numeric
value. Existing tuple, range, grouped-alternative, and unknown-member behavior
is preserved.

## Testing

- The previously disabled direct-alias regression and new chained, grouped, and
  imported-alias regressions fail before the fix and pass afterward.
- Valid grouped alternatives and distinct aliased enum members remain accepted.
- Full control-flow, exhaustiveness, and legacy typechecker suites pass.
- The enum suite passes excluding one inherited, unrelated semantic-sum
  constructor diagnostic mismatch already present on the parent branch.

pr:enum-alias-duplicate-match
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
…hrough type aliases (#31)

## Summary

- Reject duplicate enum match alternatives even when type aliases, alias
  chains, or imported aliases give the same member different source spellings.
- Preserve the existing compiler diagnostic with both original and duplicate
  source locations.
- Enable the previously disabled alias regression and cover grouped, chained,
  imported, and valid distinct-member cases.

## Problem Solved

An enum type alias does not create a new enum member, so both alternatives in
this match select the same enum member:

```dslx
type Alias = E;

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

The previous duplicate checker compared source spelling and accepted the
unreachable second arm because `E::A` and `Alias::A` are different strings.

## Implementation

Resolve enum alternatives through the existing type-alias and import resolver,
then compare their underlying enum-member declarations. Keep the existing
whole-arm and exact-spelling checks unchanged, and reuse their two-location
compiler diagnostic.

Distinct members remain distinct even when they share an underlying numeric
value. Existing tuple, range, grouped-alternative, and unknown-member behavior
is preserved.

## Testing

- The previously disabled direct-alias regression and new chained, grouped, and
  imported-alias regressions fail before the fix and pass afterward.
- Valid grouped alternatives and distinct aliased enum members remain accepted.
- Full control-flow, exhaustiveness, and legacy typechecker suites pass.
- The enum suite passes excluding one inherited, unrelated semantic-sum
  constructor diagnostic mismatch already present on the parent branch.

pr:enum-alias-duplicate-match
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
…hrough type aliases (#31)

## Summary

- Reject duplicate enum match alternatives even when type aliases, alias
  chains, or imported aliases give the same member different source spellings.
- Preserve the existing compiler diagnostic with both original and duplicate
  source locations.
- Enable the previously disabled alias regression and cover grouped, chained,
  imported, and valid distinct-member cases.

## Problem Solved

An enum type alias does not create a new enum member, so both alternatives in
this match select the same enum member:

```dslx
type Alias = E;

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

The previous duplicate checker compared source spelling and accepted the
unreachable second arm because `E::A` and `Alias::A` are different strings.

## Implementation

Resolve enum alternatives through the existing type-alias and import resolver,
then compare their underlying enum-member declarations. Keep the existing
whole-arm and exact-spelling checks unchanged, and reuse their two-location
compiler diagnostic.

Distinct members remain distinct even when they share an underlying numeric
value. Existing tuple, range, grouped-alternative, and unknown-member behavior
is preserved.

## Testing

- The previously disabled direct-alias regression and new chained, grouped, and
  imported-alias regressions fail before the fix and pass afterward.
- Valid grouped alternatives and distinct aliased enum members remain accepted.
- Full control-flow, exhaustiveness, and legacy typechecker suites pass.
- The enum suite passes excluding one inherited, unrelated semantic-sum
  constructor diagnostic mismatch already present on the parent branch.

pr:enum-alias-duplicate-match
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.
@jbtayloriii
jbtayloriii requested a review from dplassgit August 7, 2026 21:20
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