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.

Sign up for GitHub

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

Jump to bottom

Tweak impl signature mismatch errors involving RegionKind::ReVar lifetimes #67460

Merged
merged 12 commits into from
May 30, 2020
Merged

Tweak impl signature mismatch errors involving RegionKind::ReVar lifetimes #67460

merged 12 commits into from
May 30, 2020

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Dec 20, 2019 •
edited
Loading

Fix #66406, fix #72106.

error: `impl` item signature doesn't match `trait` item signature
  --> $DIR/trait-param-without-lifetime-constraint.rs:14:5
   |
LL |     fn get_relation(&self) -> To;
   |     ----------------------------- expected `fn(&Article) -> &ProofReader`
...
LL |     fn get_relation(&self) -> &ProofReader {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&Article) -> &ProofReader`
   |
   = note: expected `fn(&Article) -> &ProofReader`
              found `fn(&Article) -> &ProofReader`
help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait`
  --> $DIR/trait-param-without-lifetime-constraint.rs:10:31
   |
LL |     fn get_relation(&self) -> To;
   |                               ^^ consider borrowing this type parameter in the trait

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2019
@estebank
Copy link
Contributor Author

CC @rust-lang/wg-diagnostics we should probably have a follow up to detect these cases in particular and explain that the self lifetime is not tied to the return type's lifetime in a clearer way.

@estebank

This comment has been minimized.

Sign in to view
@estebank
Copy link
Contributor Author

New output:
Screen Shot 2019-12-22 at 1 53 29 PM
Screen Shot 2019-12-22 at 1 53 34 PM

@estebank estebank force-pushed the named-lts branch 2 times, most recently from 7bcc539 to a4967d9 Compare December 22, 2019 22:09
@bors

This comment has been minimized.

Sign in to view
@estebank estebank force-pushed the named-lts branch 2 times, most recently from 7ad6d8f to 7c00f7a Compare December 27, 2019 21:52
@estebank estebank changed the title Name RegionKind::ReVar lifetimes in diagnostics Teak impl signature mismatch errors involving RegionKind::ReVar lifetimes Dec 27, 2019
@bors

This comment has been minimized.

Sign in to view
@estebank estebank changed the title Teak impl signature mismatch errors involving RegionKind::ReVar lifetimes Tweak impl signature mismatch errors involving RegionKind::ReVar lifetimes Jan 7, 2020
@rust-highfive

This comment has been minimized.

Sign in to view
nikomatsakis
nikomatsakis reviewed Jan 8, 2020
View reviewed changes
src/test/ui/traits/trait-param-without-lifetime-constraint.stderr Outdated
--> $DIR/trait-param-without-lifetime-constraint.rs:14:5
|
LL | pub trait HaveRelationship<To> {
| -- for `impl` items to implement the method, this type parameter might need a lifetime restriction like `To: 'a`
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems sort of misleading, no?

That is, I think the proper fix here is

impl<'a> HaveRelationship<&'a ProofReader> for Article {
    fn get_relation(&self) -> &'a ProofReader {
    //~^ ERROR `impl` item signature doesn't match `trait` item signature
        &self.proof_reader
    }
}

but I'm not sure how the new error would lead me there....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that doesn't work either because HaveRelationship needs a lifetime so that get_relation takes a &'a self instead of &self. Shouldn't the correct change be?:

pub trait HaveRelationship<'a, To: 'a> {
    fn get_relation(&'a self) -> To;
}

impl<'a> HaveRelationship<'a, &'a ProofReader> for Article {
    fn get_relation(&'a self) -> &'a ProofReader {
        &self.proof_reader
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's discuss this in the issue :)

@bors
Copy link
Contributor

bors commented Feb 4, 2020

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

@estebank estebank force-pushed the named-lts branch 2 times, most recently from e2049ee to bcf34c7 Compare February 4, 2020 22:49
@bors

This comment has been minimized.

Sign in to view
@Dylan-DPC-zz Dylan-DPC-zz 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 Feb 14, 2020
@estebank estebank 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 Feb 17, 2020
@estebank estebank force-pushed the named-lts branch 2 times, most recently from 8fb857d to 63c56d4 Compare February 17, 2020 02:02
@estebank
Copy link
Contributor Author

@bors r=nikomatsakis

@nikomatsakis Approving based on https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics/topic/impl.20signature.20mismatch.20errors.20PR.20.2367460/near/198499109

@bors
Copy link
Contributor

bors commented May 28, 2020

📌 Commit f213acf has been approved by nikomatsakis

@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 May 28, 2020
@estebank
Copy link
Contributor Author

@bors r=nikomatsakis

@nikomatsakis made tiny tweak to account for Self and fix #72106.

@bors
Copy link
Contributor

bors commented May 28, 2020

📌 Commit 1bd6970 has been approved by nikomatsakis

RalfJung added a commit to RalfJung/rust that referenced this pull request May 29, 2020
@RalfJung
Rollup merge of rust-lang#67460 - estebank:named-lts, r=nikomatsakis
240e6b9 
Tweak impl signature mismatch errors involving `RegionKind::ReVar` lifetimes

Fix rust-lang#66406, fix rust-lang#72106.

```
error: `impl` item signature doesn't match `trait` item signature
  --> $DIR/trait-param-without-lifetime-constraint.rs:14:5
   |
LL |     fn get_relation(&self) -> To;
   |     ----------------------------- expected `fn(&Article) -> &ProofReader`
...
LL |     fn get_relation(&self) -> &ProofReader {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&Article) -> &ProofReader`
   |
   = note: expected `fn(&Article) -> &ProofReader`
              found `fn(&Article) -> &ProofReader`
help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait`
  --> $DIR/trait-param-without-lifetime-constraint.rs:10:31
   |
LL |     fn get_relation(&self) -> To;
   |                               ^^ consider borrowing this type parameter in the trait
```

r? @nikomatsakis
@RalfJung RalfJung mentioned this pull request May 29, 2020
Closed
RalfJung added a commit to RalfJung/rust that referenced this pull request May 29, 2020
@RalfJung
Rollup merge of rust-lang#67460 - estebank:named-lts, r=nikomatsakis
990bf40 
Tweak impl signature mismatch errors involving `RegionKind::ReVar` lifetimes

Fix rust-lang#66406, fix rust-lang#72106.

```
error: `impl` item signature doesn't match `trait` item signature
  --> $DIR/trait-param-without-lifetime-constraint.rs:14:5
   |
LL |     fn get_relation(&self) -> To;
   |     ----------------------------- expected `fn(&Article) -> &ProofReader`
...
LL |     fn get_relation(&self) -> &ProofReader {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&Article) -> &ProofReader`
   |
   = note: expected `fn(&Article) -> &ProofReader`
              found `fn(&Article) -> &ProofReader`
help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait`
  --> $DIR/trait-param-without-lifetime-constraint.rs:10:31
   |
LL |     fn get_relation(&self) -> To;
   |                               ^^ consider borrowing this type parameter in the trait
```

r? @nikomatsakis
@RalfJung RalfJung mentioned this pull request May 29, 2020
Closed
RalfJung added a commit to RalfJung/rust that referenced this pull request May 29, 2020
@RalfJung
Rollup merge of rust-lang#67460 - estebank:named-lts, r=nikomatsakis
ce10b6d 
Tweak impl signature mismatch errors involving `RegionKind::ReVar` lifetimes

Fix rust-lang#66406, fix rust-lang#72106.

```
error: `impl` item signature doesn't match `trait` item signature
  --> $DIR/trait-param-without-lifetime-constraint.rs:14:5
   |
LL |     fn get_relation(&self) -> To;
   |     ----------------------------- expected `fn(&Article) -> &ProofReader`
...
LL |     fn get_relation(&self) -> &ProofReader {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&Article) -> &ProofReader`
   |
   = note: expected `fn(&Article) -> &ProofReader`
              found `fn(&Article) -> &ProofReader`
help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait`
  --> $DIR/trait-param-without-lifetime-constraint.rs:10:31
   |
LL |     fn get_relation(&self) -> To;
   |                               ^^ consider borrowing this type parameter in the trait
```

r? @nikomatsakis
@RalfJung RalfJung mentioned this pull request May 29, 2020
Closed
@bors
Copy link
Contributor

bors commented May 29, 2020

⌛ Testing commit 1bd6970 with merge 7173e7b8fe4935c4d2567a0144469d4166541af5...

@Dylan-DPC-zz
Copy link

@bors retry yield

RalfJung added a commit to RalfJung/rust that referenced this pull request May 29, 2020
@RalfJung
Rollup merge of rust-lang#67460 - estebank:named-lts, r=nikomatsakis
7823651 
Tweak impl signature mismatch errors involving `RegionKind::ReVar` lifetimes

Fix rust-lang#66406, fix rust-lang#72106.

```
error: `impl` item signature doesn't match `trait` item signature
  --> $DIR/trait-param-without-lifetime-constraint.rs:14:5
   |
LL |     fn get_relation(&self) -> To;
   |     ----------------------------- expected `fn(&Article) -> &ProofReader`
...
LL |     fn get_relation(&self) -> &ProofReader {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&Article) -> &ProofReader`
   |
   = note: expected `fn(&Article) -> &ProofReader`
              found `fn(&Article) -> &ProofReader`
help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait`
  --> $DIR/trait-param-without-lifetime-constraint.rs:10:31
   |
LL |     fn get_relation(&self) -> To;
   |                               ^^ consider borrowing this type parameter in the trait
```

r? @nikomatsakis
@RalfJung RalfJung mentioned this pull request May 29, 2020
Closed
@RalfJung RalfJung mentioned this pull request May 29, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 29, 2020
@bors
Auto merge of rust-lang#72756 - RalfJung:rollup-tbjmtx2, r=RalfJung
0e9e408 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#67460 (Tweak impl signature mismatch errors involving `RegionKind::ReVar` lifetimes)
 - rust-lang#71095 (impl From<[T; N]> for Box<[T]>)
 - rust-lang#71500 (Make pointer offset methods/intrinsics const)
 - rust-lang#71804 (linker: Support `-static-pie` and `-static -shared`)
 - rust-lang#71862 (Implement RFC 2585: unsafe blocks in unsafe fn)
 - rust-lang#72103 (borrowck `DefId` -> `LocalDefId`)
 - rust-lang#72407 (Various minor improvements to Ipv6Addr::Display)
 - rust-lang#72413 (impl Step for char (make Range*<char> iterable))
 - rust-lang#72439 (NVPTX support for new asm!)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented May 30, 2020

☔ The latest upstream changes (presumably #72756) 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 May 30, 2020
@bors bors merged commit 8120780 into rust-lang:master May 30, 2020
@Aaron1011 Aaron1011 mentioned this pull request Aug 10, 2020
Closed
@estebank estebank deleted the named-lts branch November 9, 2023 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis left review comments

@Centril Centril Centril left review comments

Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
10 participants
@estebank @bors @rust-highfive @Centril @davidtwco @Dylan-DPC-zz @nikomatsakis @JohnCSimon @joelpalmer @Elinvynia