Overhaul `rustc_middle::limits` #136671

nnethercote · 2025-02-07T06:10:27Z

In particular, to make pattern_complexity work more like other limits, which then enables some other simplifications.

rustbot · 2025-02-07T06:10:37Z

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred in exhaustiveness checking

cc @Nadrieril

nnethercote · 2025-02-07T06:11:07Z

The last commit renames the unstable pattern_complexity attribute; I'm not sure if renaming such attributes is allowed.

cc @GuillaumeGomez

Veykril

You can't use rustc_session in rust-analyzer (and thus not in the relevant pattern matching api)

GuillaumeGomez · 2025-02-07T09:28:20Z

I don't mind the renaming, but it sounds weird "allow pattern_complexity_limits". It's supposed to make sense in a sentence. 😉

nnethercote · 2025-02-07T09:37:24Z

You can't use rustc_session in rust-analyzer (and thus not in the relevant pattern matching api)

Oh. Well, those changes are easy enough to remove, I can just use usize directly instead of importing Limit.

In what sense can't it be used? It seemed to compile ok, and a few other rustc crates are seemingly used, like rustc_index, rustc_abi, rustc_pattern_analysis. Is this a technical thing, a policy thing, something else?

Veykril · 2025-02-07T09:39:56Z

rust-analyzer needs to build on stable (as well as not depend on specifically rustc_span due to the global interning) which puts a bunch of limits onto the compiler crates it depends on (most have feature cfgs to make this work), rustc_session is not one of those (and is for example a gated dependency of rustc_pattern_analysis)

nnethercote · 2025-02-07T09:40:51Z

I don't mind the renaming, but it sounds weird "allow pattern_complexity_limits". It's supposed to make sense in a sentence. 😉

It's a crate-level attribute with a parameter, so you use it like #![pattern_complexity_limit = "1000"], just like recursion_limit. That seems better than the current #![pattern_complexity = "1000"].

GuillaumeGomez · 2025-02-07T09:42:20Z

Oh I see, makes more sense now. :)

nnethercote · 2025-02-10T08:39:48Z

Thanks for the comments and explanations. I have changed things so that rust-analyzer doesn't depend on rustc_session and CI tests are passing, and Guillaume seems happy. @Nadrieril, I think this is ready for review, thanks.

bors · 2025-02-13T00:15:36Z

☔ The latest upstream changes (presumably #136943) made this pull request unmergeable. Please resolve the merge conflicts.

compiler/rustc_pattern_analysis/tests/common/mod.rs

compiler/rustc_middle/src/middle/limits.rs

Nadrieril

Nice, I didn't know there was a shared infrastructure for limits, this is better like this. r=me once the pattern_analysis tests can also run on stable.

It's similar to the other limits, e.g. obtained via `get_limit`. So it makes sense to handle it consistently with the other limits. We now use `Limit`/`usize` in most places instead of `Option<usize>`, so we use `Limit::new(usize::MAX)`/`usize::MAX` to emulate how `None` used to work. The commit also adds `Limit::unlimited`.

Thanks to the previous commit, they no longer need to be separate.

It's always good to make `rustc_middle` smaller. `rustc_interface` is the best destination, because it's the only crate that calls `get_recursive_limit`.

For consistency with `recursion_limit`, `move_size_limit`, and `type_length_limit`.

nnethercote · 2025-02-16T22:43:21Z

I addressed the comments.

@bors r=Nadrieril rollup

bors · 2025-02-16T22:43:24Z

📌 Commit 7a8c0fc has been approved by Nadrieril

It is now in the queue for this repository.

…eril Overhaul `rustc_middle::limits` In particular, to make `pattern_complexity` work more like other limits, which then enables some other simplifications. r? `@Nadrieril`

…kingjubilee Rollup of 12 pull requests Successful merges: - rust-lang#136671 (Overhaul `rustc_middle::limits`) - rust-lang#136817 (Pattern Migration 2024: clean up and comment) - rust-lang#136844 (Use `const_error!` when possible) - rust-lang#136953 (rustc_target: import TargetMetadata) - rust-lang#137095 (Replace some u64 hashes with Hash64) - rust-lang#137100 (HIR analysis: Remove unnecessary abstraction over list of clauses) - rust-lang#137105 (Restrict DerefPure for Cow<T> impl to T = impl Clone, [impl Clone], str.) - rust-lang#137120 (Enable `relative-path-include-bytes-132203` rustdoc-ui test on Windows) - rust-lang#137125 (Re-add missing empty lines in the releases notes) - rust-lang#137140 (Fix const items not being allowed to be called `r#move` or `r#static`) - rust-lang#137145 (use add-core-stubs / minicore for a few more tests) - rust-lang#137149 (Remove SSE ABI from i586-pc-windows-msvc) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#136466 (Start removing `rustc_middle::hir::map::Map`) - rust-lang#136671 (Overhaul `rustc_middle::limits`) - rust-lang#136817 (Pattern Migration 2024: clean up and comment) - rust-lang#136844 (Use `const_error!` when possible) - rust-lang#137080 (bootstrap: add more tracing to compiler/std/llvm flows) - rust-lang#137101 (`invalid_from_utf8[_unchecked]`: also lint inherent methods) - rust-lang#137140 (Fix const items not being allowed to be called `r#move` or `r#static`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#136671 - nnethercote:middle-limits, r=Nadrieril Overhaul `rustc_middle::limits` In particular, to make `pattern_complexity` work more like other limits, which then enables some other simplifications. r? ``@Nadrieril``

rustbot assigned Nadrieril Feb 7, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 7, 2025

This comment has been minimized.

Sign in to view

Veykril reviewed Feb 7, 2025

View reviewed changes

nnethercote force-pushed the middle-limits branch from cbe77b1 to 65caf76 Compare February 10, 2025 06:21

Nadrieril reviewed Feb 16, 2025

View reviewed changes

compiler/rustc_pattern_analysis/tests/common/mod.rs Outdated Show resolved Hide resolved

Nadrieril reviewed Feb 16, 2025

View reviewed changes

compiler/rustc_middle/src/middle/limits.rs Show resolved Hide resolved

Nadrieril requested changes Feb 16, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 16, 2025

nnethercote added 5 commits February 17, 2025 09:30

Merge get_limit and get_limit_size.

30b8c84

Thanks to the previous commit, they no longer need to be separate.

Improve comments about limits.

13280ee

Move rustc_middle::limits to rustc_interface.

223c95f

It's always good to make `rustc_middle` smaller. `rustc_interface` is the best destination, because it's the only crate that calls `get_recursive_limit`.

Rename pattern_complexity attr as pattern_complexity_limit.

7a8c0fc

For consistency with `recursion_limit`, `move_size_limit`, and `type_length_limit`.

nnethercote force-pushed the middle-limits branch from 65caf76 to 7a8c0fc Compare February 16, 2025 22:40

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 16, 2025

workingjubilee mentioned this pull request Feb 16, 2025

Rollup of 12 pull requests #137153

Closed

matthiaskrgr mentioned this pull request Feb 17, 2025

Rollup of 7 pull requests #137163

Merged

bors merged commit 0c051c8 into rust-lang:master Feb 17, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Feb 17, 2025

nnethercote deleted the middle-limits branch May 21, 2025 23:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Overhaul `rustc_middle::limits` #136671

Overhaul `rustc_middle::limits` #136671

Uh oh!

nnethercote commented Feb 7, 2025

Uh oh!

rustbot commented Feb 7, 2025

Uh oh!

nnethercote commented Feb 7, 2025

Uh oh!

This comment has been minimized.

Veykril left a comment •

edited

Loading

Uh oh!

GuillaumeGomez commented Feb 7, 2025

Uh oh!

nnethercote commented Feb 7, 2025

Uh oh!

Veykril commented Feb 7, 2025

Uh oh!

nnethercote commented Feb 7, 2025

Uh oh!

GuillaumeGomez commented Feb 7, 2025

Uh oh!

nnethercote commented Feb 10, 2025

Uh oh!

bors commented Feb 13, 2025

Uh oh!

Uh oh!

Uh oh!

Nadrieril left a comment

Uh oh!

nnethercote commented Feb 16, 2025

Uh oh!

bors commented Feb 16, 2025

Uh oh!

Uh oh!

Uh oh!

Overhaul rustc_middle::limits #136671

Overhaul rustc_middle::limits #136671

Uh oh!

Conversation

nnethercote commented Feb 7, 2025

Uh oh!

rustbot commented Feb 7, 2025

Uh oh!

nnethercote commented Feb 7, 2025

Uh oh!

This comment has been minimized.

Veykril left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Feb 7, 2025

Uh oh!

nnethercote commented Feb 7, 2025

Uh oh!

Veykril commented Feb 7, 2025

Uh oh!

nnethercote commented Feb 7, 2025

Uh oh!

GuillaumeGomez commented Feb 7, 2025

Uh oh!

nnethercote commented Feb 10, 2025

Uh oh!

bors commented Feb 13, 2025

Uh oh!

Uh oh!

Uh oh!

Nadrieril left a comment

Choose a reason for hiding this comment

Uh oh!

nnethercote commented Feb 16, 2025

Uh oh!

bors commented Feb 16, 2025

Uh oh!

Uh oh!

Uh oh!

Overhaul `rustc_middle::limits` #136671

Overhaul `rustc_middle::limits` #136671

Veykril left a comment •

edited

Loading