Skip to content

Tweaks to writeback and Obligation -> Goal conversion #138846

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Mar 24, 2025

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Mar 22, 2025

Each of these commits are self-contained, but are prerequisites that I'd like to land before #138845, which still needs some cleaning.

The ""most controversial"" one is probably Explicitly don't fold coroutine obligations in writeback, which I prefer because I think using fold_predicate to control against not normalizing predicates seems... easy to mess up 🤔, and we could have other things that we don't want to normalize.

Explicitly noting whether we want resolve to normalize is a lot clearer (and currently in writeback is limited to resolving stalled coroutine obligations), since we can attach it to a comment that explains why.

@compiler-errors
Copy link
Member Author

r? lcnr

@rustbot
Copy link
Collaborator

rustbot commented Mar 22, 2025

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Mar 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 22, 2025

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

changes to inspect_obligations.rs

cc @compiler-errors, @lcnr

@rustbot rustbot assigned lcnr and unassigned Nadrieril Mar 22, 2025
self.handle_term(ct, ty::Const::outer_exclusive_binder, |tcx, guar| {
ty::Const::new_error(tcx, guar)
})
.super_fold_with(self)
Copy link
Member Author

Choose a reason for hiding this comment

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

No need to super fold b/c it should be fully resolved :)

@compiler-errors compiler-errors changed the title Stall prereqs Tweaks to writeback and Obligation -> Goal conversion Mar 23, 2025
self.typeck_results.offset_of_data_mut().insert(hir_id, (container, indices.clone()));
}
}

fn resolve<T>(&mut self, value: T, span: &dyn Locatable) -> T
fn resolve<T>(&mut self, value: T, span: &dyn Locatable, should_normalize: bool) -> T
Copy link
Contributor

Choose a reason for hiding this comment

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

having a bool argument for this seems confusing.

Please either change this to a custom enum or split this into (normalize_and_)resolve and resolve_delayed_obligations

Copy link
Member Author

Choose a reason for hiding this comment

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

i think normalize_and_resolve + resolve_delayed_coroutine_obligations is fine. a bit of code duplication, but I also dislike all the , true additions.

@lcnr
Copy link
Contributor

lcnr commented Mar 23, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 23, 2025

📌 Commit fad34c6 has been approved by lcnr

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 Mar 23, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#137593 (fix download-llvm logic for subtree sync branches)
 - rust-lang#137736 (Don't attempt to export compiler-builtins symbols from rust dylibs)
 - rust-lang#138135 (Simplify `PartialOrd` on tuples containing primitives)
 - rust-lang#138321 ([bootstrap] Distribute split debuginfo if present)
 - rust-lang#138574 (rustdoc: be more strict about "Methods from Deref")
 - rust-lang#138606 (Fix missing rustfmt in msi installer - cont)
 - rust-lang#138671 (Fix `FileType` `PartialEq` implementation on Windows)
 - rust-lang#138728 (Update `compiler-builtins` to 0.1.152)
 - rust-lang#138783 (Cache current_dll_path output)
 - rust-lang#138846 (Tweaks to writeback and `Obligation -> Goal` conversion)

Failed merges:

 - rust-lang#138755 ([rustdoc] Remove duplicated loop when computing doc cfgs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0e95f96 into rust-lang:master Mar 24, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2025
Rollup merge of rust-lang#138846 - compiler-errors:stall-prereqs, r=lcnr

Tweaks to writeback and `Obligation -> Goal` conversion

Each of these commits are self-contained, but are prerequisites that I'd like to land before rust-lang#138845, which still needs some cleaning.

The ""most controversial"" one is probably [Explicitly don't fold coroutine obligations in writeback](rust-lang@e7d27ba), which I prefer because I think using `fold_predicate` to control against not normalizing predicates seems... easy to mess up 🤔, and we could have *other things* that we don't want to normalize.

Explicitly noting whether we want `resolve` to normalize is a lot clearer (and currently in writeback is limited to resolving stalled coroutine obligations), since we can attach it to a comment that explains *why*.
@rustbot rustbot added this to the 1.87.0 milestone Mar 24, 2025
# for free to join this conversation on GitHub. Already have an account? # 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.

5 participants