Skip to content

do not unnecessarily leak auto traits in item bounds #139789

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

Merged
merged 3 commits into from
Apr 15, 2025

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Apr 14, 2025

fixes rust-lang/trait-system-refactor-initiative#158

Not a fix for rust-lang/trait-system-refactor-initiative#173 as you may have realized/tried yourself, cc #139788. However, fixing this feels desirable regardless and I don't see any reason not to.

r? @compiler-errors

@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 Apr 14, 2025
@lcnr lcnr force-pushed the opaques-auto-trait-leakage branch from 21c52ae to 48d9f26 Compare April 14, 2025 09:30
@@ -0,0 +1,31 @@
//@ revisions: current next
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be marked as a known-bug.

// To avoid this we don't try to leak auto trait bounds if they can also be proven via
// item bounds of the opaque. These bounds are always applicable as auto traits must not
// have any generic parameters. They would also get preferred over the impl candidate
// when merging candidates anyways.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe find some place to also explicitly mention that this is sound b/c we later separately check that the hidden type implements the RPIT's bounds after opaque type inference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this feels like a more general question about alias bound candidates of opaques 🤔 so we'd probably have to put it there. To be totally honest, I am not confident that it is fully sound in the first place as long as checking well-formedness for opaque types happens in analysis mode xx

I've kept #109387 in my github notifications for multiple months now

@lcnr lcnr force-pushed the opaques-auto-trait-leakage branch from 7f4bc99 to 2ed2d65 Compare April 14, 2025 11:03
@rust-log-analyzer

This comment has been minimized.

@lcnr lcnr force-pushed the opaques-auto-trait-leakage branch from 2ed2d65 to 836ea25 Compare April 14, 2025 11:26
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 14, 2025

📌 Commit 836ea25 has been approved by compiler-errors

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 14, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 15, 2025
…=compiler-errors

do not unnecessarily leak auto traits in item bounds

fixes rust-lang/trait-system-refactor-initiative#158

Not a fix for rust-lang/trait-system-refactor-initiative#173 as you may have realized/tried yourself, cc rust-lang#139788. However, fixing this feels desirable regardless and I don't see any reason not to.

r? `@compiler-errors`
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang#138374 (Enable contracts for const functions)
 - rust-lang#138380 (ci: add runners for vanilla LLVM 20)
 - rust-lang#138393 (Allow const patterns of matches to contain pattern types)
 - rust-lang#139393 (rustdoc-json: Output target feature information)
 - rust-lang#139517 (std: sys: process: uefi: Use NULL stdin by default)
 - rust-lang#139554 (std: add Output::exit_ok)
 - rust-lang#139745 (Avoid unused clones in `Cloned<I>` and `Copied<I>`)
 - rust-lang#139757 (opt-dist: use executable-extension for host llvm-profdata)
 - rust-lang#139778 (Add test for issue 34834)
 - rust-lang#139783 (Use `compiletest-ignore-dir` for bootstrap self-tests)
 - rust-lang#139789 (do not unnecessarily leak auto traits in item bounds)
 - rust-lang#139791 (drop global where-bounds before merging candidates)

r? `@ghost`
`@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Apr 15, 2025
…=compiler-errors

do not unnecessarily leak auto traits in item bounds

fixes rust-lang/trait-system-refactor-initiative#158

Not a fix for rust-lang/trait-system-refactor-initiative#173 as you may have realized/tried yourself, cc rust-lang#139788. However, fixing this feels desirable regardless and I don't see any reason not to.

r? ``@compiler-errors``
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
Rollup of 17 pull requests

Successful merges:

 - rust-lang#138374 (Enable contracts for const functions)
 - rust-lang#138380 (ci: add runners for vanilla LLVM 20)
 - rust-lang#138393 (Allow const patterns of matches to contain pattern types)
 - rust-lang#139517 (std: sys: process: uefi: Use NULL stdin by default)
 - rust-lang#139554 (std: add Output::exit_ok)
 - rust-lang#139660 (compiletest: Add an experimental new executor to replace libtest)
 - rust-lang#139669 (Overhaul `AssocItem`)
 - rust-lang#139671 (Proc macro span API redesign: Replace proc_macro::SourceFile by Span::{file, local_file})
 - rust-lang#139750 (std/thread: Use default stack size from menuconfig for NuttX)
 - rust-lang#139772 (Remove `hir::Map`)
 - rust-lang#139785 (Let CStrings be either 1 or 2 byte aligned.)
 - rust-lang#139789 (do not unnecessarily leak auto traits in item bounds)
 - rust-lang#139791 (drop global where-bounds before merging candidates)
 - rust-lang#139798 (normalize: prefer `ParamEnv` over `AliasBound` candidates)
 - rust-lang#139822 (Fix: Map EOPNOTSUPP to ErrorKind::Unsupported on Unix)
 - rust-lang#139833 (Fix some HIR pretty-printing problems)
 - rust-lang#139836 (Basic tests of MPMC receiver cloning)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8118fca into rust-lang:master Apr 15, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 15, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
Rollup merge of rust-lang#139789 - lcnr:opaques-auto-trait-leakage, r=compiler-errors

do not unnecessarily leak auto traits in item bounds

fixes rust-lang/trait-system-refactor-initiative#158

Not a fix for rust-lang/trait-system-refactor-initiative#173 as you may have realized/tried yourself, cc rust-lang#139788. However, fixing this feels desirable regardless and I don't see any reason not to.

r? ```@compiler-errors```
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 19, 2025
Rollup of 17 pull requests

Successful merges:

 - rust-lang#138374 (Enable contracts for const functions)
 - rust-lang#138380 (ci: add runners for vanilla LLVM 20)
 - rust-lang#138393 (Allow const patterns of matches to contain pattern types)
 - rust-lang#139517 (std: sys: process: uefi: Use NULL stdin by default)
 - rust-lang#139554 (std: add Output::exit_ok)
 - rust-lang#139660 (compiletest: Add an experimental new executor to replace libtest)
 - rust-lang#139669 (Overhaul `AssocItem`)
 - rust-lang#139671 (Proc macro span API redesign: Replace proc_macro::SourceFile by Span::{file, local_file})
 - rust-lang#139750 (std/thread: Use default stack size from menuconfig for NuttX)
 - rust-lang#139772 (Remove `hir::Map`)
 - rust-lang#139785 (Let CStrings be either 1 or 2 byte aligned.)
 - rust-lang#139789 (do not unnecessarily leak auto traits in item bounds)
 - rust-lang#139791 (drop global where-bounds before merging candidates)
 - rust-lang#139798 (normalize: prefer `ParamEnv` over `AliasBound` candidates)
 - rust-lang#139822 (Fix: Map EOPNOTSUPP to ErrorKind::Unsupported on Unix)
 - rust-lang#139833 (Fix some HIR pretty-printing problems)
 - rust-lang#139836 (Basic tests of MPMC receiver cloning)

r? `@ghost`
`@rustbot` modify labels: rollup
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.

opaque type auto trait leakage is used even when item bounds would be sufficient
5 participants