Skip to content

Fix query cycle when encounter unevaluated const - #148698

Merged
bors merged 2 commits into
rust-lang:mainfrom
tiif:const_query_cycle
Nov 17, 2025
Merged

Fix query cycle when encounter unevaluated const#148698
bors merged 2 commits into
rust-lang:mainfrom
tiif:const_query_cycle

Conversation

@tiif

@tiiftiif commented Nov 8, 2025

Copy link
Copy Markdown
Member

Fixesrust-lang/trait-system-refactor-initiative#249

In this PR, the environment is dropped when evaluating const that does not have any generic parameter to fix the query cycle.

@rustbotrustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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 Nov 8, 2025
@tiif

tiif commented Nov 8, 2025•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

I didn't do this for the GCE and RepeatExprCount path above for now because I don't think they will have the same query cycle problem? 🤔 If that should be done, feel free to let me know, and it'd be nice to have test for those paths too.

@tiif

tiif commented Nov 8, 2025

Copy link
Copy Markdown
MemberAuthor

r? @BoxyUwU

@rustbot

rustbot commented Nov 8, 2025

Copy link
Copy Markdown
Collaborator

BoxyUwU is currently at their maximum review capacity.
They may take a while to respond.

@tiif
tiif marked this pull request as ready for review November 8, 2025 16:31
@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 8, 2025
BoxyUwU
BoxyUwU reviewed Nov 10, 2025
View reviewed changes
Comment threadcompiler/rustc_trait_selection/src/traits/mod.rs Outdated

// Since there is no generic parameter, we can just drop the environment
// to prevent query cycle.
if !uv.args.has_non_region_param() {

@BoxyUwUBoxyUwUNov 10, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you should be able to do this unconditionally because we early exit above if there are any generic parameters. can can you try always using the empty env. should also be able to get rid of the previous assignment to typing_env this way too

@tiiftiifNov 11, 2025•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

If there is any region param / region infer in args, unconditionally doing this will empty everything in param_env though, is it ok?

@tiiftiifNov 11, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

As discussed privately, it is ok because region will be erased later.

@BoxyUwUBoxyUwU 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 Nov 10, 2025
@tiiftiif added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 11, 2025
@BoxyUwU

BoxyUwU commented Nov 17, 2025

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 17, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 10fa334 has been approved by BoxyUwU

It is now in the queue for this repository.

@borsbors 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 Nov 17, 2025
@matthiaskrgrmatthiaskrgr mentioned this pull request Nov 17, 2025
Merged
bors added a commit that referenced this pull request Nov 17, 2025
@bors
Auto merge of#149032- matthiaskrgr:rollup-28xhwxa, r=matthiaskrgr
0df64c5
Rollup of 5 pull requests
Successful merges:
- #145610 (Stabilize `char_max_len`)
- #148504 (Fix link in c_longlong documentation)
- #148698 (Fix query cycle when encounter unevaluated const)
- #148865 (move GAT inference prevention hack)
- #149016 (Document the `let this = self;` idiom used in MIR building)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 3bc1eaa into rust-lang:mainNov 17, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 17, 2025
rust-timer added a commit that referenced this pull request Nov 17, 2025
@rust-timer
Unrolled build for#148698
e4f389e
Rollup merge of #148698 - tiif:const_query_cycle, r=BoxyUwU
Fix query cycle when encounter unevaluated const
Fixesrust-lang/trait-system-refactor-initiative#249
In this PR, the environment is dropped when evaluating const that does not have any generic parameter to fix the query cycle.
lcnr
lcnr reviewed Nov 19, 2025
View reviewed changes
Comment threadcompiler/rustc_trait_selection/src/traits/mod.rs
.with_post_analysis_normalized(tcx);
// Since there is no generic parameter, we can just drop the environment
// to prevent query cycle.
let typing_env = infcx.typing_env(ty::ParamEnv::empty());

@lcnrlcnrNov 19, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

that typing env is wrong 🤔 surprised this doesn't blow up. We should use ty::TypingEnv::post_analysis here

I expect us to currently not normalize opaques or default assoc items because of that

@BoxyUwUBoxyUwUNov 19, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yh it seemed kinda funny to me too, I was surprised we don't have a test for this

@tiiftiifNov 19, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

oops will fix this soon. do y'all have any test in mind that could surface this issue in future?

@lcnrlcnrNov 19, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

some use of a TAIT while evaluating the constant. I don't quite know what would cause it to fail 🤔 I guess maybe calling a method on the opaque?

@tiiftiifNov 21, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Fixed in #149183, didn't manage to craft a test that can make it fail :(

github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
@bors
Auto merge of #149032 - matthiaskrgr:rollup-28xhwxa, r=matthiaskrgr
896c6e3
Rollup of 5 pull requests
Successful merges:
- rust-lang/rust#145610 (Stabilize `char_max_len`)
- rust-lang/rust#148504 (Fix link in c_longlong documentation)
- rust-lang/rust#148698 (Fix query cycle when encounter unevaluated const)
- rust-lang/rust#148865 (move GAT inference prevention hack)
- rust-lang/rust#149016 (Document the `let this = self;` idiom used in MIR building)
r? `@ghost`
`@rustbot` modify labels: rollup
@tiiftiif mentioned this pull request Nov 21, 2025
Merged
Zalathar added a commit to Zalathar/rust that referenced this pull request Dec 4, 2025
@Zalathar
Rollup merge ofrust-lang#149183- tiif:typing_env_fix, r=BoxyUwU
b6f0bf7
Use `TypingMode::PostAnalysis` in `try_evaluate_const`
As mentioned in rust-lang#148698 (comment), we should use ``TypingMode::PostAnalysis`` for that path.
`@BoxyUwU` prefer the match in ``try_evaluate_const`` to be exhaustive, so I also included that in this PR :3
Zalathar added a commit to Zalathar/rust that referenced this pull request Dec 4, 2025
@Zalathar
Rollup merge ofrust-lang#149183- tiif:typing_env_fix, r=BoxyUwU
d2f56d0
Use `TypingMode::PostAnalysis` in `try_evaluate_const`
As mentioned in rust-lang#148698 (comment), we should use ``TypingMode::PostAnalysis`` for that path.
``@BoxyUwU`` prefer the match in ``try_evaluate_const`` to be exhaustive, so I also included that in this PR :3
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 4, 2025
@matthiaskrgr
Rollup merge ofrust-lang#149183- tiif:typing_env_fix, r=BoxyUwU
ef73bbe
Use `TypingMode::PostAnalysis` in `try_evaluate_const`
As mentioned in rust-lang#148698 (comment), we should use ``TypingMode::PostAnalysis`` for that path.
```@BoxyUwU``` prefer the match in ``try_evaluate_const`` to be exhaustive, so I also included that in this PR :3
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 4, 2025
@matthiaskrgr
Rollup merge ofrust-lang#149183- tiif:typing_env_fix, r=BoxyUwU
bea2f89
Use `TypingMode::PostAnalysis` in `try_evaluate_const`
As mentioned in rust-lang#148698 (comment), we should use ``TypingMode::PostAnalysis`` for that path.
````@BoxyUwU```` prefer the match in ``try_evaluate_const`` to be exhaustive, so I also included that in this PR :3
rust-timer added a commit that referenced this pull request Dec 4, 2025
@rust-timer
Unrolled build for#149183
44a9083
Rollup merge of #149183 - tiif:typing_env_fix, r=BoxyUwU
Use `TypingMode::PostAnalysis` in `try_evaluate_const`
As mentioned in #148698 (comment), we should use ``TypingMode::PostAnalysis`` for that path.
````@BoxyUwU```` prefer the match in ``try_evaluate_const`` to be exhaustive, so I also included that in this PR :3
github-actionsBot pushed a commit to rust-lang/rust-analyzer that referenced this pull request Dec 15, 2025
@matthiaskrgr
Rollup merge of #149183 - tiif:typing_env_fix, r=BoxyUwU
df184e7
Use `TypingMode::PostAnalysis` in `try_evaluate_const`
As mentioned in rust-lang/rust#148698 (comment), we should use ``TypingMode::PostAnalysis`` for that path.
````@BoxyUwU```` prefer the match in ``try_evaluate_const`` to be exhaustive, so I also included that in this PR :3
@lcnrlcnr mentioned this pull request Dec 15, 2025
Open
8 tasks
ghaaj pushed a commit to ghaaj/polygrammar that referenced this pull request Aug 7, 2026
@matthiaskrgr
Rollup merge of #149183 - tiif:typing_env_fix, r=BoxyUwU
96b7a42
Use `TypingMode::PostAnalysis` in `try_evaluate_const`
As mentioned in rust-lang/rust#148698 (comment), we should use ``TypingMode::PostAnalysis`` for that path.
````@BoxyUwU```` prefer the match in ``try_evaluate_const`` to be exhaustive, so I also included that in this PR :3
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BoxyUwUBoxyUwUBoxyUwU left review comments
@lcnrlcnrlcnr left review comments

Assignees

@BoxyUwUBoxyUwU

Labels

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

Projects

None yet

Milestone

1.93.0

Development

Successfully merging this pull request may close these issues.

unevaluated consts in param_env query cycle

5 participants

@tiif@rustbot@BoxyUwU@bors@lcnr