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

Extra trait bound makes function body fail to typecheck #82219

Closed
jplatte opened this issue Feb 17, 2021 · 13 comments
Closed

Extra trait bound makes function body fail to typecheck #82219

jplatte opened this issue Feb 17, 2021 · 13 comments
Labels
A-traits Area: Trait system C-bug Category: This is a bug. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@jplatte
Copy link
Contributor

jplatte commented Feb 17, 2021

I tried this code (playground):

pub trait Foo<'a> {}

impl Foo<'_> for () {}
impl<'a, T> Foo<'a> for Option<T> where T: Foo<'a> {}

pub fn bar<T>()
where
    T: for<'a> Foo<'a>,
{
}

fn baz<'a>()
where
    (): Foo<'a>,
{
    bar::<Option<()>>();
}
Or with references instead of Option
impl<'a, 'b, T> Foo<'a> for &'b T where T: Foo<'a> {}
bar::<&()>();

I expected to see this happen: Compile successfully, with or without the unneeded (): Foo<'a> bound.

Instead, this happened: It works when the (): Foo<'a> bound is removed, but as written it produces:

error: implementation of `Foo` is not general enough
  --> src/lib.rs:16:5
   |
1  | pub trait Foo<'a> {}
   | -------------------- trait `Foo` defined here
...
16 |     bar::<Option<()>>();
   |     ^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough
   |
   = note: `Option<()>` must implement `Foo<'0>`, for any lifetime `'0`...
   = note: ...but `Option<()>` actually implements `Foo<'1>`, for some specific lifetime `'1`

Meta

rustc --version --verbose:

rustc 1.50.0 (cb75ad5db 2021-02-10)
binary: rustc
commit-hash: cb75ad5db02783e8b0222fee363c5f63f7e2cf5b
commit-date: 2021-02-10
host: x86_64-unknown-linux-gnu
release: 1.50.0

Also happens on Nightly.

@jplatte
Copy link
Contributor Author

jplatte commented Aug 23, 2021

@rustbot label: +A-traits

@rustbot rustbot added the A-traits Area: Trait system label Aug 23, 2021
@BoxyUwU BoxyUwU added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 2, 2021
@BoxyUwU
Copy link
Member

BoxyUwU commented Sep 6, 2021

theres a zulip topic in wg-traits about this here

@bsodmike
Copy link

bsodmike commented Feb 7, 2022

I'm on rustc 1.59.0-beta.5 (28c8a34e1 2022-01-27) and am getting this as well,

error: implementation of `sqlx::Decode` is not general enough
   --> src/models.rs:183:5
    |
183 |     Decode,
    |     ^^^^^^ implementation of `sqlx::Decode` is not general enough
    |
    = note: `std::string::String` must implement `sqlx::Decode<'0, sqlx::Postgres>`, for some specific lifetime `'0`...
    = note: ...but it actually implements `sqlx::Decode<'r, sqlx::Postgres>`
    = note: this error originates in the derive macro `Decode` (in Nightly builds, run with -Z macro-backtrace for more info)

@grv07
Copy link

grv07 commented Dec 6, 2022

I am on 1.65 and still getting this :(

image

@frederikhors
Copy link

Is there a way to close this? This is very huge.

@MirrorBytes
Copy link

It's been 3 years and this issue is still occurring on 1.76.0. Not even higher-ranked trait bounds fixes this; applying them causes another set of errors, and after correcting them, that ultimately leads right back to this one.

@BoxyUwU BoxyUwU added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 31, 2024
@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 31, 2024

I suspect we will never fix this and it should be closed as wontfix, the FCP in #119820 means that we never intend to consider the region constraint 'a eq !'0 during candidate assembly for the goal so I don't think we'll ever have a way to choose the impl candidate over the param candidate.

In theory (under the new solver) the proof tree for the issue description should look somewhat like:

  • for<'a> Option<()>: Foo<'a>
    • Option<()>: Foo<'?0> // instantiating the goal with placeholders then canonicalizing it results in all regions being infers
      • Impl Candidate Option<?0>: Foo<'?1>
        • which has a nested goal of ?0: Foo<'?1>
      • Param Candidate Option<()>: Foo<'?1>

Both of these candidates will have results of Ok(Yes) with external constraints for equating '?0 from the goal with the in the candidate. The region constraint from the param candidate (which does not actually hold) is only checked in the root for<'a> Option<()>: Foo<'a> goal after we have already completed candidate assembly for the Option<()>: Foo<'?0> goal.

Going to just copy the thing we did with #121602 and do an fcp to close this incase anyone on t-types thinks there's a reasonable way to support this that does not involve going back on our decision in #119820.
@rfcbot fcp close

(as a sidenote the bump on this thread is hilarious timing as I intended to write this fcp close comment once I got back from rust nation but had not gotten the chance to find this issue again so the notification was perfect for saving me time doing that lol)

@rfcbot
Copy link

rfcbot commented Mar 31, 2024

Team member @BoxyUwU has proposed to close this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels Mar 31, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Mar 31, 2024

We should keep this issue open for improving the diagnostics (or open a new one)

Also not sure whether we have a test. Should add one, too

@lcnr
Copy link
Contributor

lcnr commented Apr 2, 2024

This could get theoretically fixed by switching to an entirely eager approach to region unification/constraints.

I don't expect that to happen any time soon (next ~5 years) so I believe wontfix to be appropriate.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Apr 23, 2024
@rfcbot
Copy link

rfcbot commented Apr 23, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@jplatte
Copy link
Contributor Author

jplatte commented Apr 24, 2024

I came across a seemingly related issue with many many more linked issues yesterday: #24066 (via this blog post: https://veera.app/trait_selection_bug.html)
If this is not fixable in the next years, does the same apply there?

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 3, 2024
@rfcbot
Copy link

rfcbot commented May 3, 2024

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label May 3, 2024
@compiler-errors compiler-errors closed this as not planned Won't fix, can't repro, duplicate, stale May 3, 2024
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system C-bug Category: This is a bug. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests