Skip to content

Resolve some cases of #132279 by using the right typing mode in the next solver#156141

Open
jdonszelmann wants to merge 3 commits intorust-lang:mainfrom
jdonszelmann:use-right-typingmode
Open

Resolve some cases of #132279 by using the right typing mode in the next solver#156141
jdonszelmann wants to merge 3 commits intorust-lang:mainfrom
jdonszelmann:use-right-typingmode

Conversation

@jdonszelmann
Copy link
Copy Markdown
Contributor

@jdonszelmann jdonszelmann commented May 4, 2026

r? @lcnr

Convert 3 FIXMEs of #132279 to using the right typing mode when we can (tcx.use_typing_mode_borrowck())

Also resolves #155093, which I closed

@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. labels May 4, 2026
@jdonszelmann jdonszelmann force-pushed the use-right-typingmode branch from bac5eb5 to 7d76533 Compare May 4, 2026 13:28
@jdonszelmann jdonszelmann marked this pull request as ready for review May 4, 2026 13:28
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 4, 2026

Some changes occurred to constck

cc @fee1-dead

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@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 May 4, 2026
Comment on lines 103 to 105
// FIXME(#132279): Once we've got a typing mode which reveals opaque types using the HIR
// typeck results without causing query cycles, we should use this here instead of defining
// opaque types.
Copy link
Copy Markdown
Contributor

@lcnr lcnr May 4, 2026

Choose a reason for hiding this comment

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

do remove the fixme please

View changes since the review

}
} else {
match self.phase {
// FIXME(#132279): we should reveal the opaques defined in the body during analysis.
Copy link
Copy Markdown
Contributor

@lcnr lcnr May 4, 2026

Choose a reason for hiding this comment

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

I think that fixme can also be removed 🤷 there's nothing actionable here

View changes since the review

Comment on lines +324 to +328
// Defuse the drop bomb in the OpaqueTypeStorage when we're in TypingMode::Borrowck,
// and the InferCtxt doesn't consider regions. This is okay since in `Borrowck`,
// the only reason we care about opaques is in relation to regions.
// In some places *after* typeck, like in lints we use `TypingMode::Borrowck`
// to prevent defining opaque types and we simply don't care about regions.
Copy link
Copy Markdown
Contributor

@lcnr lcnr May 4, 2026

Choose a reason for hiding this comment

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

want to move the Drop check the opaque_type_storage to here and remove that impl, having the handling of this assert split over 2 impls feels suboptimal

View changes since the review

@lcnr
Copy link
Copy Markdown
Contributor

lcnr commented May 4, 2026

@rustbot author

@rustbot 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 May 4, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 4, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants