Skip to content

Conversation

@davidwrighton
Copy link
Member

Fix issue where switch instructions did not properly handle dispatch to blocks which already had defined their incoming stack slots

…dle dispatch to blocks which already had defined their incoming stack slots
Copilot AI review requested due to automatic review settings October 15, 2025 17:57
@davidwrighton davidwrighton changed the title [clr-interp] Fix swtich statement stack slot handling [clr-interp] Fix switch statement stack slot handling Oct 15, 2025
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 15, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fixes incorrect stack slot handling for switch instructions when multiple case labels branch to the same target basic block. Key changes ensure per-target stack state initialization and variable move emission are performed only once per unique target.

  • Added collection of target offsets and deduplication via sorting before emitting per-target setup.
  • Moved INTOP_SWITCH instruction emission until after target block preparation.

@davidwrighton davidwrighton merged commit d3c6f06 into dotnet:main Oct 17, 2025
101 checks passed
stephentoub added a commit that referenced this pull request Oct 17, 2025
This PR ports functional regex tests from the <a
href="https://github.com/google/re2/tree/main/re2/testing">RE2 test
suite</a> to improve .NET's regex test coverage, as requested in
#120756.

## Changes

### Test Suite Additions
- **Added `RegexRe2Tests.cs`**: New test file containing 85 unique test
cases ported from RE2's `re2_test.cc` and `search_test.cc`
- **Test coverage**: The ported tests execute ~340 times across all
available regex engines (Interpreter, Compiled, NonBacktracking,
SourceGenerated)
- **Removed duplicates**: Analyzed overlap with existing PCRE, Rust, and
core regex tests; removed 57 duplicative test cases to avoid redundancy

### Test Categories Covered
The ported tests validate:
- Complex matching patterns and alternations
- Anchors (`^`, `$`) in single-line and multiline modes with non-trivial
cases
- Word boundaries (`\b`, `\B`) with ASCII and special characters
- UTF-8/Unicode character handling
- Escape sequences (octal `\141`, hexadecimal `\x61`, unicode `\u0061`)
- Case-insensitive matching (`(?i)`)
- Non-trivial quantifier combinations (`{n}`, `{n,}`, `{n,m}`)
- Backreferences (excluded for NonBacktracking engine)
- Edge cases and historical bug patterns from RE2

### Compatibility Adjustments
Several RE2-specific patterns were excluded or adapted for .NET
compatibility:
- **Removed `\C` patterns**: RE2's byte-matching construct is not
supported in .NET
- **Removed `\Q...\E` patterns**: Quote meta syntax is not supported in
.NET
- **Converted `\x{...}` escapes**: Changed to .NET-compatible `\x` (2
digits) or `\u` (4 digits) format
- **Excluded backreferences for NonBacktracking**: Uses
`RegexHelpers.IsNonBacktracking()` check
- **Removed Unicode word boundary test**: .NET treats Unicode letters as
word characters (unlike RE2/PCRE which use ASCII-only)
- **Removed duplicative tests**: Excluded basic patterns like "a", "a*",
"a+", "a?", "^$" that are already well-covered in existing test files

### License Attribution
- **Updated `THIRD-PARTY-NOTICES.TXT`**: Added RE2 BSD 3-Clause license
to properly attribute the ported tests
- **Updated `.csproj`**: Added `RegexRe2Tests.cs` to the compilation

## Testing
All 29,621 tests pass successfully, including the newly ported RE2 tests
across all regex engines.

Fixes #120756

<!-- START COPILOT CODING AGENT SUFFIX -->



<details>

<summary>Original prompt</summary>

> 
> ----
> 
> *This section details on the original issue you should resolve*
> 
> <issue_title>Port re2 regex tests</issue_title>
> <issue_description>re2 has lots of regex functional tests we should
port over to improve our coverage:
>
https://github.com/google/re2/blob/d826d9fcb68c62996c1b7c0a45d604e22d814952/re2/testing/</issue_description>
> 
> <agent_instructions>You can see in
https://github.com/dotnet/runtime/tree/main/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests
we have a file of tests ported from PCRE and a file of tests ported from
Rust. Please similarly port the cited RE2 tests (or a newer version if
there is one); any that don't pass but should need to be marked as
[ActiveIssue(...)]... any that don't pass but that's expected should be
removed.</agent_instructions>
> 
> ## Comments on the Issue (you are @copilot in this section)
> 
> <comments>
> <comment_new><author>@</author><body>
> Tagging subscribers to this area: @eerhardt,
@dotnet/area-system-text-regularexpressions
> See info in
[area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md)
if you want to be subscribed.
> <details>
> <summary>Issue Details</summary>
> <hr />
> 
> re2 has lots of regex functional tests we should port over to improve
our coverage:
>
https://github.com/google/re2/blob/d826d9fcb68c62996c1b7c0a45d604e22d814952/re2/testing/
> 
> <table>
>   <tr>
>     <th align="left">Author:</th>
>     <td>stephentoub</td>
>   </tr>
>   <tr>
>     <th align="left">Assignees:</th>
>     <td>-</td>
>   </tr>
>   <tr>
>     <th align="left">Labels:</th>
>     <td>
> 
> `area-System.Text.RegularExpressions`
> 
> </td>
>   </tr>
>   <tr>
>     <th align="left">Milestone:</th>
>     <td>.NET 7.0</td>
>   </tr>
> </table>
> </details></body></comment_new>
> <comment_new><author>@joperezr</author><body>
> This will likely miss ZBB but will still make it before 7.0. Adjusting
the milestone just because this is not technically a blocker for
7.0.</body></comment_new>
> </comments>
> 


</details>

Fixes #61896

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions,
customizing its development environment and configuring Model Context
Protocol (MCP) servers. Learn more [Copilot coding agent
tips](https://gh.io/copilot-coding-agent-tips) in the docs.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants