Skip to content

limit the api of fold_predicate and visit_predicate - #162461

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
jdonszelmann:restrict-fold-predicate
Sep 8, 2026
Merged

limit the api of fold_predicate and visit_predicate#162461
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
jdonszelmann:restrict-fold-predicate

Conversation

@jdonszelmann

Copy link
Copy Markdown
Contributor

r? @lcnr or anyone in @rust-lang/initiative-trait-system-refactor

In the future, we may want to start compressing clauses in the ParamEnv. One major problem was that we are leaking too many implementation details in fold_predicate and visit_predicate. Almost no code actually cares about dealing with an actual predicate there. Instead, what really matters is the type flags on a predicate for example. As such, this PR majorly limits the API that is exposed to a folder, hiding the underlying predicate data structure used.

As an example why this matters: in the case of compressed clauses, this will mean we won't need to "decompress" them. Instead, we can just fold over the self type, not telling folders whether the predicate was or was not compressed at all.

Note

I've not used an LLM for any part of this PR, or any other PR I make. This includes any related work like research.

@rustbot 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Sep 8, 2026
@BoxyUwU

BoxyUwU commented Sep 8, 2026

Copy link
Copy Markdown
Member

what does compressing clauses mean :3

@jdonszelmann

jdonszelmann commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

haha, fair point: there's a plan to reduce the size of ParamEnvs, by representing very common trait clauses as bitsets: <type>: Sized | MetaSized | Move | Forget | Clone | Copy | Debug (for example). This should, fingers crossed, take care of a lot of the blowup when adding new auto traits, like Move

@BoxyUwU

BoxyUwU commented Sep 8, 2026

Copy link
Copy Markdown
Member

ah sick yeah that makes sense, follow up question, how far off is "in the future"? If this is just a first step in work that is actively ongoing to do that then this seems reasonable to me 🤔 If we have no active plans to do the perf work here this feels kinda meh to me as it makes the folder logic more abstract and harder to reason about.

@jdonszelmann

jdonszelmann commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

So I already have a prototype. We're still struggling a little, since it's essential for performance not to decompress these clauses all over the place. Previously, in the folders, we were simply lying and folding over only the self type, skipping fold_predicate. This is correct, ish, for the specific traits we had, and since no folder currently in rustc cares too much about fold_predicate. However, by doing so, we're essentially just hoping no folder in the future will care about this, and this PR adds the type safety for that.

In other words, this is the first step towards doing compression, something which is currently in the works, and this is its own PR since is mergeable as is.

Comment thread compiler/rustc_type_ir/src/fold.rs
@BoxyUwU

BoxyUwU commented Sep 8, 2026

Copy link
Copy Markdown
Member

will just r? me :3

@rustbot rustbot assigned BoxyUwU and unassigned lcnr Sep 8, 2026
@jdonszelmann
jdonszelmann force-pushed the restrict-fold-predicate branch from 24be2a8 to cb34499 Compare September 8, 2026 11:55
@rustbot

rustbot commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

changes to the core type system

cc @lcnr

@jdonszelmann
jdonszelmann force-pushed the restrict-fold-predicate branch from cb34499 to 8622679 Compare September 8, 2026 12:13
@rustbot

rustbot commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@BoxyUwU

BoxyUwU commented Sep 8, 2026

Copy link
Copy Markdown
Member

@bors r+ rollup

@rust-bors

rust-bors Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 8622679 has been approved by BoxyUwU

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

Reason for tree closure: spurious failures

@rust-bors rust-bors Bot 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 8, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 8, 2026
…te, r=BoxyUwU

limit the api of `fold_predicate` and `visit_predicate`

r? @lcnr or anyone in @rust-lang/initiative-trait-system-refactor

In the future, we may want to start compressing clauses in the `ParamEnv`. One major problem was that we are leaking too many implementation details in `fold_predicate` and `visit_predicate`. Almost no code actually cares about dealing with an actual predicate there. Instead, what really matters is the type flags on a predicate for example. As such, this PR majorly limits the API that is exposed to a folder, hiding the underlying predicate data structure used.

As an example why this matters: in the case of compressed clauses, this will mean we won't need to "decompress" them. Instead, we can just fold over the self type, not telling folders whether the predicate was or was not compressed at all.

> [!NOTE]
> I've not used an LLM for any part of this PR, or any other PR I make. This includes any related work like research.
rust-bors Bot pushed a commit that referenced this pull request Sep 8, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #162309 (offload: automate manual clang-linker-wrapper step)
 - #160505 (delegation: supporting inherent impls)
 - #160712 (windows-gnullvm: always link libunwind statically)
 - #161423 (trait_selection: Keep type-op region constraints in borrowck)
 - #162461 (limit the api of `fold_predicate` and `visit_predicate`)
 - #162475 (Fix unsoundness bug on next trait solver for dyn const generics placeholder)
@rust-bors
rust-bors Bot merged commit d8dd9f1 into rust-lang:main Sep 8, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 8, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 8, 2026
Rollup merge of #162461 - jdonszelmann:restrict-fold-predicate, r=BoxyUwU

limit the api of `fold_predicate` and `visit_predicate`

r? @lcnr or anyone in @rust-lang/initiative-trait-system-refactor

In the future, we may want to start compressing clauses in the `ParamEnv`. One major problem was that we are leaking too many implementation details in `fold_predicate` and `visit_predicate`. Almost no code actually cares about dealing with an actual predicate there. Instead, what really matters is the type flags on a predicate for example. As such, this PR majorly limits the API that is exposed to a folder, hiding the underlying predicate data structure used.

As an example why this matters: in the case of compressed clauses, this will mean we won't need to "decompress" them. Instead, we can just fold over the self type, not telling folders whether the predicate was or was not compressed at all.

> [!NOTE]
> I've not used an LLM for any part of this PR, or any other PR I make. This includes any related work like research.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants