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

type alias impl trait: add tests showing that hidden type only outlives lifetimes that occur in bounds #103876

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Nov 2, 2022

fixes #103642

#102417 only made sure that hidden types cannot outlive lifetimes other than the ones mentioned on bounds, but didn't allow us to actually infer anything from that.

cc @aliemjay

@rustbot
Copy link
Collaborator

rustbot commented Nov 2, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

Please see the contribution instructions for more information.

@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 2, 2022
@oli-obk oli-obk changed the title type alias impl trait: only require hidden type to outlive lifetimes if they occur in bounds type alias impl trait: assume hidden type only outlives lifetimes that occur in bounds Nov 2, 2022
@aliemjay
Copy link
Member

aliemjay commented Nov 2, 2022

We also need a similar fix for outlive components. For example this should also pass:

// ... same test as #102417
fn want_static<T: 'static>(_: T) {}

fn test<'a>() {
    want_static(<&'a () as Callable>::call());
}

Now with all these non-local workarounds I wonder if the generics should instead be modified during HIR lowering or in the query generics_of as discussed in RPIT refactoring meeting.

Thanks for the pull request, and welcome!

lol why rustbot assumes you are a new contributor?

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 2, 2022

We also need a similar fix for outlive components.

yea, that's what I looked at first, but couldn't actually hit it. Thanks for building a repro

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 2, 2022

Now with all these non-local workarounds I wonder if the generics should instead be modified during HIR lowering or in the query generics_of as discussed in RPIT refactoring meeting.

oh definitely. That's actually what I tried first in #102417, but that is very hard at present. We basically need #96840 first in order to be able to do this nicely.

compiler/rustc_infer/src/infer/opaque_types.rs Outdated Show resolved Hide resolved
compiler/rustc_infer/src/infer/opaque_types.rs Outdated Show resolved Hide resolved
compiler/rustc_infer/src/infer/opaque_types.rs Outdated Show resolved Hide resolved
compiler/rustc_infer/src/infer/outlives/obligations.rs Outdated Show resolved Hide resolved
let z = y.foo();
let _a = &x; // invalidate the `&'a mut`in `y`
//~^ ERROR cannot borrow `x`
let _b = z; // this should *not* check that `'a` in the type `Foo<'a>::foo::opaque` is live
Copy link
Contributor

@lcnr lcnr Nov 14, 2022

Choose a reason for hiding this comment

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

that comment seems wrong? If you have FooX<'a> with an explicit 'a the error feels correct? 🤔

can you add a comment to the top of the file explaining what we're testing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that comment seems wrong? If you have FooX<'a> with an explicit 'a the error feels correct?

that's the thing... lifetimes on type aliases should not matter for anything. Only lifetimes in the aliased type. Once/if we avoid adding uncaptured lifetimes to opaque types, this would automatically end up compiling. It may end up working with #103491 or with that plus some small changes.

@bors
Copy link
Contributor

bors commented Nov 21, 2022

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

@oli-obk oli-obk changed the title type alias impl trait: assume hidden type only outlives lifetimes that occur in bounds type alias impl trait: add tests showing that hidden type only outlives lifetimes that occur in bounds Nov 22, 2022
@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 22, 2022

This entire PR has become unnecessary with #103491, so all it does now is add tests

@lcnr
Copy link
Contributor

lcnr commented Nov 29, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 29, 2022

📌 Commit ca57832 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 Nov 29, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2022
…iaskrgr

Rollup of 14 pull requests

Successful merges:

 - rust-lang#103876 (type alias impl trait: add tests showing that hidden type only outlives lifetimes that occur in bounds)
 - rust-lang#104427 (Explain why `rematch_impl` fails to be infallible)
 - rust-lang#104436 (Add slice to the stack allocated string comment)
 - rust-lang#104523 (Don't use periods in target names)
 - rust-lang#104627 (Print all features with --print target-features)
 - rust-lang#104911 (Make inferred_outlives_crate return Clause)
 - rust-lang#105002 (Add `PathBuf::as_mut_os_string` and `Path::as_mut_os_str`)
 - rust-lang#105023 (Statics used in reachable function's inline asm are reachable)
 - rust-lang#105045 (`rustc_ast_{passes,pretty}`: remove `ref` patterns)
 - rust-lang#105049 (Hermit: Minor build fixes)
 - rust-lang#105051 (Replace a macro with a function)
 - rust-lang#105062 (rustdoc: use shorthand background for rustdoc toggle CSS)
 - rust-lang#105066 (move `candidate_from_obligation` out of assembly)
 - rust-lang#105068 (Run patchelf also on rust-analyzer-proc-macro-srv.)

Failed merges:

 - rust-lang#105050 (Remove useless borrows and derefs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3617adf into rust-lang:master Nov 30, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 30, 2022
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.

TAIT still captures lifetime generics implicitely
5 participants