Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes by stephentoub · Pull Request #101608 · dotnet/runtime

stephentoub · 2024-04-26T14:48:27Z

For an expression like [Aa]{2}, we were generating the strings "AA" and "aa" but not "Aa" or "aA".

This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

dotnet-policy-service · 2024-04-26T14:48:59Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

danmoseley · 2024-04-26T15:57:46Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

stephentoub · 2024-04-26T16:02:23Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

The code is already there. It should be correct.

…refixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

…refixes (dotnet#101608) * Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled. * Also exit early as soon as we can detect too many possible prefixes

stephentoub · 2024-04-26T14:48:27Z

For an expression like [Aa]{2}, we were generating the strings "AA" and "aa" but not "Aa" or "aA".

This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

dotnet-policy-service · 2024-04-26T14:48:59Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

danmoseley · 2024-04-26T15:57:46Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

stephentoub · 2024-04-26T16:02:23Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

The code is already there. It should be correct.

…refixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

…refixes (dotnet#101608) * Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled. * Also exit early as soon as we can detect too many possible prefixes

stephentoub · 2024-04-26T14:48:27Z

For an expression like [Aa]{2}, we were generating the strings "AA" and "aa" but not "Aa" or "aA".

This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

dotnet-policy-service · 2024-04-26T14:48:59Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

danmoseley · 2024-04-26T15:57:46Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

stephentoub · 2024-04-26T16:02:23Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

The code is already there. It should be correct.

…refixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

…refixes (dotnet#101608) * Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled. * Also exit early as soon as we can detect too many possible prefixes

stephentoub · 2024-04-26T14:48:27Z

For an expression like [Aa]{2}, we were generating the strings "AA" and "aa" but not "Aa" or "aA".

This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

dotnet-policy-service · 2024-04-26T14:48:59Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

danmoseley · 2024-04-26T15:57:46Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

stephentoub · 2024-04-26T16:02:23Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

The code is already there. It should be correct.

…refixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

…refixes (dotnet#101608) * Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled. * Also exit early as soon as we can detect too many possible prefixes

stephentoub · 2024-04-26T14:48:27Z

For an expression like [Aa]{2}, we were generating the strings "AA" and "aa" but not "Aa" or "aA".

This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

dotnet-policy-service · 2024-04-26T14:48:59Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

danmoseley · 2024-04-26T15:57:46Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

stephentoub · 2024-04-26T16:02:23Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

The code is already there. It should be correct.

…refixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

…refixes (dotnet#101608) * Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled. * Also exit early as soon as we can detect too many possible prefixes

stephentoub · 2024-04-26T14:48:27Z

For an expression like [Aa]{2}, we were generating the strings "AA" and "aa" but not "Aa" or "aA".

This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

dotnet-policy-service · 2024-04-26T14:48:59Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

danmoseley · 2024-04-26T15:57:46Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

stephentoub · 2024-04-26T16:02:23Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

The code is already there. It should be correct.

…refixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

…refixes (dotnet#101608) * Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled. * Also exit early as soon as we can detect too many possible prefixes

stephentoub · 2024-04-26T14:48:27Z

For an expression like [Aa]{2}, we were generating the strings "AA" and "aa" but not "Aa" or "aA".

This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

dotnet-policy-service · 2024-04-26T14:48:59Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

danmoseley · 2024-04-26T15:57:46Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

stephentoub · 2024-04-26T16:02:23Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

The code is already there. It should be correct.

…refixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

…refixes (dotnet#101608) * Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled. * Also exit early as soon as we can detect too many possible prefixes

stephentoub · 2024-04-26T14:48:27Z

For an expression like [Aa]{2}, we were generating the strings "AA" and "aa" but not "Aa" or "aA".

This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

dotnet-policy-service · 2024-04-26T14:48:59Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

danmoseley · 2024-04-26T15:57:46Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

stephentoub · 2024-04-26T16:02:23Z

Perhaps I'm not following, but wouldn't we normally avoid making a change that isn't exercised, and instead include it with the change that uses it?

The code is already there. It should be correct.

…refixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled.

…refixes (dotnet#101608) * Fix handling of case-sensitive set loops in RegexPrefixAnalyzer.FindPrefixes For an expression like `[Aa]{2}`, we were generating the strings "AA" and "aa" but not "Aa" or "aA". This code isn't exercised yet, as we're currently only using FindPrefixes for case-insensitive, but I'm trying to enable it for case-sensitive as well, and hit this. I'm not adding new tests here as plenty of existing tests catch it once it's enabled. * Also exit early as soon as we can detect too many possible prefixes

stephentoub requested a review from MihaZupan April 26, 2024 14:48

ghost added the area-System.Text.RegularExpressions label Apr 26, 2024

dotnet-policy-serviceBot assigned stephentoub Apr 26, 2024

stephentoub force-pushed the fixsetloops branch from 0b4bd8f to e77256fCompare April 26, 2024 16:34

Also exit early as soon as we can detect too many possible prefixes
db16aa9

stephentoub force-pushed the fixsetloops branch from e77256f to db16aa9Compare April 29, 2024 13:07

build-analysisBot mentioned this pull request Apr 29, 2024
[wasm][firefox] crit: OpenQA.Selenium.WebDriverException: Failed to decode response from marionette #101617
Open
3 tasks

MihaZupan approved these changes Apr 29, 2024
View reviewed changes

stephentoub merged commit 73a428d into dotnet:mainApr 29, 2024

stephentoub deleted the fixsetloops branch April 29, 2024 17:55

github-actionsBot locked and limited conversation to collaborators May 31, 2024