Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fuel to match checking #16879

Merged
merged 4 commits into from
Mar 19, 2024
Merged

Add fuel to match checking #16879

merged 4 commits into from
Mar 19, 2024

Conversation

Nadrieril
Copy link
Member

Exhaustiveness checking is NP-hard hence can take extremely long to check some specific matches. This PR makes ehxaustiveness bail after a set number of steps. I chose a bound that takes ~100ms on my machine, which should be more than enough for normal matches.

I'd like someone with less recent hardware to run the test to see if that limit is low enough for them. Also curious if the r-a team thinks this is a good ballpark or if we should go lower/higher. I don't have much data on how complex real-life matches get, but we can definitely go lower than 500 000 steps.

The second commit is a drive-by soundness fix which doesn't matter much today but will matter once min_exhaustive_patterns is stabilized.

Fixes #9528 cc @matklad

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 18, 2024
@Veykril
Copy link
Member

Veykril commented Mar 19, 2024

Running time cargo test -p ide-diagnostics exponential_match --release gives me ~344ms (debug is more like 2 seconds). I think that should be fine. Might be good to add a

    if skip_slow_tests() {
        return;
    }

in front given usually the test runs in debug. With that it gets skipped by default when people run the test suite (while CI will still always run it).

@Nadrieril
Copy link
Member Author

Nadrieril commented Mar 19, 2024

I'll make the test even bigger actually, to be sure it hangs CI if we regress ^^

@Veykril
Copy link
Member

Veykril commented Mar 19, 2024

Thanks 👍
@bors r+

@bors
Copy link
Contributor

bors commented Mar 19, 2024

📌 Commit 08a5f1e has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 19, 2024

⌛ Testing commit 08a5f1e with merge a2f73d3...

@bors
Copy link
Contributor

bors commented Mar 19, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing a2f73d3 to master...

@bors bors merged commit a2f73d3 into rust-lang:master Mar 19, 2024
11 checks passed
@Nadrieril Nadrieril deleted the fuel branch March 19, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add fuel to match checking
4 participants