Skip to content

Fix query cycle when encounter unevaluated const#148698

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

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

Conversation

@tiif
Copy link
Member

@tiif tiif commented Nov 8, 2025

Fixes rust-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.

@rustbot rustbot 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
Copy link
Member Author

tiif commented Nov 8, 2025 •
edited
Loading

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
Copy link
Member Author

tiif commented Nov 8, 2025

r? @BoxyUwU

@rustbot
Copy link
Collaborator

rustbot commented Nov 8, 2025

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
@rustbot rustbot 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
compiler/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() {
Copy link
Member

@BoxyUwU BoxyUwU Nov 10, 2025

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

Copy link
Member Author

@tiif tiif Nov 11, 2025 •
edited
Loading

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?

Copy link
Member Author

@tiif tiif Nov 11, 2025

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.

@BoxyUwU BoxyUwU 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
@tiif tiif 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
Copy link
Member

BoxyUwU commented Nov 17, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 17, 2025

📌 Commit 10fa334 has been approved by BoxyUwU

It is now in the queue for this repository.

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 17, 2025
@matthiaskrgr matthiaskrgr 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:main Nov 17, 2025
11 checks passed
@rustbot rustbot 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

Fixes rust-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
compiler/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());
Copy link
Contributor

@lcnr lcnr Nov 19, 2025

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

Copy link
Member

@BoxyUwU BoxyUwU Nov 19, 2025

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

Copy link
Member Author

@tiif tiif Nov 19, 2025

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?

Copy link
Contributor

@lcnr lcnr Nov 19, 2025

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?

Copy link
Member Author

@tiif tiif Nov 21, 2025

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-actions bot 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
@tiif tiif 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 of rust-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 of rust-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 of rust-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 of rust-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-actions bot 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
@lcnr lcnr mentioned this pull request Dec 15, 2025
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BoxyUwU BoxyUwU BoxyUwU left review comments

@lcnr lcnr lcnr left review comments

Assignees

@BoxyUwU BoxyUwU

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

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