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

Confusing impl Trait limitation #104526

Closed
c410-f3r opened this issue Nov 17, 2022 · 10 comments
Closed

Confusing impl Trait limitation #104526

c410-f3r opened this issue Nov 17, 2022 · 10 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@c410-f3r
Copy link
Contributor

c410-f3r commented Nov 17, 2022

For some reason, impl Trait is allowed in a type's first mention but not in a where clause.

use core::ops::Add;

// Allowed
fn foo<T: Add<Output = impl Default>>(t: T) {}
fn bar<T: AsRef<impl Default>>(t: T) {}

// Not allowed
fn another_foo<T>(t: T)
where
    T: Add<Output = impl Default>,
{
}
fn another_bar<T>(t: T)
where
    T: AsRef<impl Default>,
{
}

Besides, the error message isn't very helpful.

`impl Trait` only allowed in function and inherent method return types, not in *****bound***** (which bound?)
@Rageking8
Copy link
Contributor

@rustbot label +T-compiler +A-diagnostics D-confusing

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 17, 2022
@cjgillot cjgillot added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-help-wanted Call for participation: Help is requested to fix this issue. labels Nov 17, 2022
@cjgillot
Copy link
Contributor

There are 2 stages to fixing this issue. Everything happens in rustc_ast_lowering.

  1. Make the diagnostic more explicit. The diagnostic is named MisplacedImplTrait. The term "bound" comes from the Display impl for ImplTraitPosition.
  2. Allow impl-trait in where clauses. We'd need to investigate a bit why lowering marks impl-trait as disallowed in one case and not the other.

@jonathanCogan
Copy link
Contributor

@rustbot claim

@estebank
Copy link
Contributor

A wording change and an applicable suggestion to write something like this would be great.

@bombless
Copy link
Contributor

bombless commented Nov 27, 2022

What does "only allowed in function and inherent method return types" actually mean?
In the original RFC, it says "may only be written within the return type of a freestanding or inherent-impl function, not in trait definitions or any non-return type position", but it is not the case now per RFC1951 and related PR.
So I think this "inherent method return types" phrase should be removed entirely.
Further more what does "only allowed in function" mean anyway?

According to RFC1951 the major compilation error we deal with here should be "ERROR impl Trait not allowed within Fn trait sugar".

All RFC1522-related tests seems to be outdated, e.g. ui/impl-trait/where-allowed.rs.

I'm trying to address above-mentioned issues, so discussion above is what I have thought of so far, I'm writing it down in case I miss anything.

@jonathanCogan
Copy link
Contributor

I think that the current message means “only allowed in function [return types] and inherent method return types” but I agree that it is no longer correct because impl Trait is allowed in the argument position, and there are plans to expand it to more locations.

@estebank
Copy link
Contributor

TBH, it should suggest adding a type param.

@bombless
Copy link
Contributor

https://github.com/rust-lang/rust/blob/master/src/test/ui/impl-trait/where-allowed.rs#L13
we are already allowing this form, it's just inconsistent.
So suggesting adding a type param might be a regression

@c410-f3r
Copy link
Contributor Author

Closing due to the lack of activity

@Jules-Bertholet
Copy link
Contributor

This is being tracked in rust-lang/impl-trait-initiative#15 and rust-lang/impl-trait-initiative#16, I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants