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

Account for late-bound depth when capturing all opaque lifetimes. #132466

Merged
merged 3 commits into from
Nov 2, 2024

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Nov 1, 2024

@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. labels Nov 1, 2024
@compiler-errors
Copy link
Member

Can you please add this as a check-pass test:

use std::future::Future;

trait Test {
    fn foo<'a>(&'a self) -> Box<dyn Future<Output = impl IntoIterator<Item = u32>>> {
        Box::new(async { [] })
    }
}

fn main() {}

@compiler-errors
Copy link
Member

Actually, just delete that other test. That one is not as minimized as the one I shared above, which also reproduces this ICE but doesn't result in an unnecessary type error.

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 2, 2024

📌 Commit e2a50de 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 Nov 2, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 2, 2024
…rrors

Account for late-bound depth when capturing all opaque lifetimes.

Fixes rust-lang#132429

r? `@compiler-errors`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 2, 2024
…rrors

Account for late-bound depth when capturing all opaque lifetimes.

Fixes rust-lang#132429

r? ``@compiler-errors``
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 2, 2024
…rrors

Account for late-bound depth when capturing all opaque lifetimes.

Fixes rust-lang#132429

r? ```@compiler-errors```
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 2, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#131037 (Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`)
 - rust-lang#132170 (Add a Few Codegen Tests)
 - rust-lang#132333 (rust_analyzer_helix.toml: add library/ manifest)
 - rust-lang#132398 (Add a couple of intra-doc links to str)
 - rust-lang#132411 (CI: switch dist-x86_64-musl to free runner)
 - rust-lang#132453 (Also treat `impl` definition parent as transparent regarding modules)
 - rust-lang#132457 (Remove needless #![feature(asm_experimental_arch)] from loongarch64 inline assembly test)
 - rust-lang#132465 (refactor(config): remove FIXME statement in comment of `omit-git-hash`)
 - rust-lang#132466 (Account for late-bound depth when capturing all opaque lifetimes.)
 - rust-lang#132471 (Add a bunch of mailmap entries)
 - rust-lang#132488 (Remove or fix some more `FIXME(async_closure)`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 020b63a into rust-lang:master Nov 2, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 2, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 2, 2024
Rollup merge of rust-lang#132466 - cjgillot:opaque-late, r=compiler-errors

Account for late-bound depth when capturing all opaque lifetimes.

Fixes rust-lang#132429

r? ````@compiler-errors````
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 2, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#131037 (Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`)
 - rust-lang#132398 (Add a couple of intra-doc links to str)
 - rust-lang#132453 (Also treat `impl` definition parent as transparent regarding modules)
 - rust-lang#132457 (Remove needless #![feature(asm_experimental_arch)] from loongarch64 inline assembly test)
 - rust-lang#132465 (refactor(config): remove FIXME statement in comment of `omit-git-hash`)
 - rust-lang#132466 (Account for late-bound depth when capturing all opaque lifetimes.)
 - rust-lang#132471 (Add a bunch of mailmap entries)
 - rust-lang#132488 (Remove or fix some more `FIXME(async_closure)`)

r? `@ghost`
`@rustbot` modify labels: rollup
@cjgillot cjgillot deleted the opaque-late branch November 2, 2024 13:36
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 16, 2024
Deny capturing late-bound ty/const params in nested opaques

First, this reverts a7f6095. I can't exactly remember why I approved this specific bit of rust-lang#132466; specifically, I don't know that the purpose of that commit is, and afaict we will never have an opaque that captures late-bound params through a const because opaques can't be used inside of anon consts. Am I missing something `@cjgillot?` Since I can't see a case where this matters, and no tests seem to fail.

The second commit adds a `deny_late_regions: bool` to distinguish `Scope::LateBoundary` which should deny *any* late-bound params or just ty/consts. Then, when resolving opaques we wrap ourselves in a `Scope::LateBoundary { deny_late_regions: false }` so that we deny late-bound ty/const, which fixes a bunch of ICEs that all vaguely look like `impl for<T> Trait<Assoc = impl OtherTrait<T>>`.

I guess this could be achieved other ways; for example, with a different scope kind, or maybe we could just reuse `Scope::Opaque`. But this seems a bit more verbose. I'm open to feedback anyways.

Fixes rust-lang#131535
Fixes rust-lang#131637
Fixes rust-lang#132530

I opted to remove those crashes tests ^ without adding them as regular tests, since they're basically triggering uninteresting late-bound ICEs far off in the trait solver, and the reason that existing tests such as `tests/ui/type-alias-impl-trait/non-lifetime-binder-in-constraint.rs` don't ICE are kinda just coincidental (i.e. due to a missing impl block). I don't really feel motivated to add random permutations to tests just to exercise non-lifetime binders.

r? cjgillot
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2024
Rollup merge of rust-lang#132832 - compiler-errors:late-ty, r=cjgillot

Deny capturing late-bound ty/const params in nested opaques

First, this reverts a7f6095. I can't exactly remember why I approved this specific bit of rust-lang#132466; specifically, I don't know that the purpose of that commit is, and afaict we will never have an opaque that captures late-bound params through a const because opaques can't be used inside of anon consts. Am I missing something `@cjgillot?` Since I can't see a case where this matters, and no tests seem to fail.

The second commit adds a `deny_late_regions: bool` to distinguish `Scope::LateBoundary` which should deny *any* late-bound params or just ty/consts. Then, when resolving opaques we wrap ourselves in a `Scope::LateBoundary { deny_late_regions: false }` so that we deny late-bound ty/const, which fixes a bunch of ICEs that all vaguely look like `impl for<T> Trait<Assoc = impl OtherTrait<T>>`.

I guess this could be achieved other ways; for example, with a different scope kind, or maybe we could just reuse `Scope::Opaque`. But this seems a bit more verbose. I'm open to feedback anyways.

Fixes rust-lang#131535
Fixes rust-lang#131637
Fixes rust-lang#132530

I opted to remove those crashes tests ^ without adding them as regular tests, since they're basically triggering uninteresting late-bound ICEs far off in the trait solver, and the reason that existing tests such as `tests/ui/type-alias-impl-trait/non-lifetime-binder-in-constraint.rs` don't ICE are kinda just coincidental (i.e. due to a missing impl block). I don't really feel motivated to add random permutations to tests just to exercise non-lifetime binders.

r? cjgillot
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE: assertion failed: value <= 0xFFFF_FF00
5 participants