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

Equate fn outputs when inferring RPITIT hidden types #101614

Merged
merged 1 commit into from
Sep 10, 2022

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Sep 9, 2022

When we are trying to infer the hidden types for RPITITs, we need to equate the output tys instead of just subtyping them. For example:

trait Foo { fn bar() -> impl Sized {} }

impl Foo for () { fn bar() -> &'static str { "" } }

If we just subtype the signatures fn() -> &'static str <: fn() -> _#1t (where _#1t is the variable we've used to infer impl Sized), we'll end up &'static str <: _#1t, which causes us to infer _#1t = #'_#2r str, where '_#2r is unconstrained, which gets fixed up to ReEmpty, and which is certainly not what we want.

I can't actually think of a way to make this fail to compile, because during borrowck we've already done the method probe, and so we just look at the impl method signature and see the &'static str any time we call <() as Foo>::bar(). But this does cause the ICE here in @jackh726's "Remove ReEmpty" PR (#98559) to stop ICEing, because after that PR we were leaking unconstrained region variables into the typeck results.

r? types

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 9, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 9, 2022
@compiler-errors compiler-errors added the F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` label Sep 9, 2022
@jackh726
Copy link
Member

jackh726 commented Sep 9, 2022

It's interesting. This is a place where a higher-ranked for<'a> &'a str probably makes sense. I'm trying to imagine what else we could expect this to be. 'static str obviously "works", but could be limiting if there are multiple hidden types. But maybe it's right because it's in the function signature, i.e. we're saying exactly what this signature is.

Anyways, this hack is fine for now. r=me with this issue linked in the comment

@compiler-errors
Copy link
Member Author

@bors r=jackh726

@bors
Copy link
Contributor

bors commented Sep 9, 2022

📌 Commit 022e3fe has been approved by jackh726

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 Sep 9, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 10, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#101578 (remove bound var hack in `resolve`)
 - rust-lang#101606 (doc: fix minor typo)
 - rust-lang#101614 (Equate fn outputs when inferring RPITIT hidden types)
 - rust-lang#101631 (rustdoc: avoid cleaning modules with duplicate names)
 - rust-lang#101635 (Move `Queries::new` out of the macro)
 - rust-lang#101641 (Update browser-ui-test version to 0.9.8)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f1412a6 into rust-lang:master Sep 10, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 10, 2022
@jackh726 jackh726 mentioned this pull request Sep 10, 2022
@compiler-errors compiler-errors deleted the rpitit-eq branch November 2, 2022 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` 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.

6 participants