Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Stall auto trait assembly in new solver for int/float vars #109752

Merged

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Mar 30, 2023

Make sure that we don't match int/float vars against all manual auto trait impls due to this check:

// This differs from the current stable behavior and
// fixes #84857. Due to breakage found via crater, we
// currently instead lint patterns which can be used to
// exploit this unsoundness on stable, see #93367 for
// more details.
//
// Using `TreatProjections::NextSolverLookup` is fine here because
// `instantiate_constituent_tys_for_auto_trait` returns nothing for
// projection types anyways. So it doesn't really matter what we do
// here, and this is faster.
if let Some(def_id) = ecx.tcx().find_map_relevant_impl(
goal.predicate.def_id(),
goal.predicate.self_ty(),
TreatProjections::NextSolverLookup,
Some,
) {
debug!(?def_id, ?goal, "disqualified auto-trait implementation");
return Err(NoSolution);
}

Since find_map_relevant_impl treats all impls as candidates for int/float vars, due to the way that fast_reject::simplify_type works.

This fixes compiler-errors/next-solver-hir-issues#11.

r? @lcnr

@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 30, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 30, 2023

Some changes occurred to the core trait solver

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

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

can you instead make this is a match on goal.predicate.self_ty().kind() and only call find_map_relevant_impl for actual rigid types, already a bit worried about how this handles alias types and what not.

r=me afterwards

@lcnr lcnr 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 Mar 31, 2023
@rust-cloud-vms rust-cloud-vms bot force-pushed the new-solver-stall-auto-trait-for-num-var branch 2 times, most recently from ea14918 to 0b7cae5 Compare April 3, 2023 23:39
@compiler-errors
Copy link
Member Author

I'd actually probably like a re-review on this, just in case I'm implementing what @lcnr wanted.

@rustbot ready

@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 Apr 3, 2023
@lcnr
Copy link
Contributor

lcnr commented Apr 10, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 10, 2023

📌 Commit 0b7cae532dff143bbf722ff92bfe1b65c4227f53 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 Apr 10, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2023
Rollup of 6 pull requests

Successful merges:

 - rust-lang#109724 (prioritize param env candidates if they don't guide type inference)
 - rust-lang#110021 (Fix a couple ICEs in the new `CastKind::Transmute` code)
 - rust-lang#110044 (Avoid some manual slice length calculation)
 - rust-lang#110115 (compiletest: Use remap-path-prefix only in CI)
 - rust-lang#110121 (Fix `x check --stage 1` when download-rustc is enabled)
 - rust-lang#110124 (Some clippy fixes in the compiler)

Failed merges:

 - rust-lang#109752 (Stall auto trait assembly in new solver for int/float vars)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Apr 10, 2023

☔ The latest upstream changes (presumably #110137) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 10, 2023
@rust-cloud-vms rust-cloud-vms bot force-pushed the new-solver-stall-auto-trait-for-num-var branch from 0b7cae5 to 8d2dbba Compare April 10, 2023 16:00
@compiler-errors
Copy link
Member Author

@bors r=lcnr

@bors
Copy link
Contributor

bors commented Apr 10, 2023

📌 Commit 8d2dbba 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 10, 2023
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Apr 11, 2023
…uto-trait-for-num-var, r=lcnr

Stall auto trait assembly in new solver for int/float vars

Make sure that we don't match int/float vars against *all* manual auto trait impls due to this check:

https://github.com/rust-lang/rust/blob/2fb0e8d162a021f8a795fb603f5d8c0017855160/compiler/rustc_trait_selection/src/solve/trait_goals.rs#L151-L169

Since `find_map_relevant_impl` treats all impls as candidates for int/float vars, due to the way that `fast_reject::simplify_type` works.

This fixes compiler-errors/next-solver-hir-issues#11.

r? `@lcnr`
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2023
Rollup of 8 pull requests

Successful merges:

 - rust-lang#109527 (Set up standard library path substitution in rust-gdb and gdbgui)
 - rust-lang#109752 (Stall auto trait assembly in new solver for int/float vars)
 - rust-lang#109860 (Add support for RISC-V relax target feature)
 - rust-lang#109923 (Update `error [E0449]: unnecessary visibility qualifier` to be more clear)
 - rust-lang#110070 (The `wrapping_neg` example for unsigned types shouldn't use `i8`)
 - rust-lang#110146 (fix(doc): do not parse inline when output is json for external crate)
 - rust-lang#110147 (Add regression test for rust-lang#104916)
 - rust-lang#110149 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a69cc45 into rust-lang:master Apr 11, 2023
@rustbot rustbot added this to the 1.70.0 milestone Apr 11, 2023
@compiler-errors compiler-errors deleted the new-solver-stall-auto-trait-for-num-var branch August 11, 2023 20:10
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.

{integer} isn't considered Send
4 participants