-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Standardize "use parentheses to call" suggestions between typeck and trait selection #102863
Standardize "use parentheses to call" suggestions between typeck and trait selection #102863
Conversation
r? @nagisa (rust-highfive has picked a reviewer for you, use r? to override) |
@@ -11,6 +11,10 @@ note: required by a bound in `take_const_owned` | |||
| | |||
LL | fn take_const_owned<F>(_: F) where F: FnOnce() + Sync + Send { | |||
| ^^^^ required by this bound in `take_const_owned` | |||
help: use parentheses to call this type parameter | |||
| | |||
LL | take_const_owned(f()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is, unfortunately, an erroneous suggestion that is uncovered by the generalizations I made in the last commit in the stack. However, this is not really the fault of the changes I made. For example, a similarly erroneous suggestion already triggers on nightly with this code:
struct Foo;
fn foo() -> Foo {
todo!()
}
trait Bar {}
impl Bar for Foo {}
fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }
fn main() {
needs_foo_and_callable(foo);
}
Results in:
error[E0277]: the trait bound `fn() -> Foo {foo}: Bar` is not satisfied
--> src/main.rs:14:15
|
3 | fn foo() -> Foo {
| --- consider calling this function
...
14 | needs_foo_and_callable(foo);
| ---------------------- ^^^ the trait `Bar` is not implemented for fn item `fn() -> Foo {foo}`
| |
| required by a bound introduced by this call
|
note: required by a bound in `needs_foo_and_callable`
--> src/main.rs:11:25
|
11 | fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }
| ^^^ required by this bound in `needs_foo`
help: use parentheses to call the function
|
14 | needs_foo_and_callable(foo());
| ++
But adding those parentheses causes:
error[E0277]: expected a `Fn<()>` closure, found `Foo`
--> src/main.rs:14:15
|
14 | needs_foo_and_callable(foo());
| ---------------------- ^^^^^ expected an `Fn<()>` closure, found `Foo`
| |
| required by a bound introduced by this call
|
= help: the trait `Fn<()>` is not implemented for `Foo`
= note: wrap the `Foo` in a closure with no arguments: `|| { /* code */ }`
note: required by a bound in `needs_foo_and_callable`
--> src/main.rs:11:31
|
11 | fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }
| ^^^^^^^^^ required by this bound in `needs_foo`
For more information about this error, try `rustc --explain E0277`.
This is a fundamental limitation to the suggestion being implemented here in trait selection -- I'm happy to brainstorm ways of suppressing this, but I'm happy with keeping it as is and filing a follow-up bug.
src/test/ui/suggestions/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.stderr
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me, r=me with the wording adjusted.
Somewhat unfortunate that we end up copying a large chunk of code outta typeck but I don’t see a good way to avoid it 🤷
| | ||
LL | fn insert_resource<R: Resource>(resource: R) {} | ||
| ^^^^^^^^ required by this bound in `insert_resource` | ||
help: use parentheses to instantiate this tuple struct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not entirely sure about using “instantiate” here, due to the parallels with OOP. I think we typically tend towards using “construct”.
1b53537
to
13f16ad
Compare
This comment has been minimized.
This comment has been minimized.
13f16ad
to
35f1570
Compare
@bors r=nagisa |
Rollup of 6 pull requests Successful merges: - rust-lang#102863 (Standardize "use parentheses to call" suggestions between typeck and trait selection) - rust-lang#103034 (Let expressions on RHS shouldn't be terminating scopes) - rust-lang#103127 (Make transpose const and inline) - rust-lang#103153 (Allow `Vec::leak` when using `no_global_oom_handling`) - rust-lang#103182 (Clean up query descriptions) - rust-lang#103216 (Consider patterns in fn params in an `Elided(Infer)` lifetime rib.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
FnDef
s but they have a different def kind and hir representation, so we were leaving them out./* Ty */
as the placeholder for an arg, and not the parameter's name, which is less helpful.predicate_must_hold_modulo_regions
instead of matching onEvaluationResult
-- this might cause some suggestions to be filtered out, but we really shouldn't be suggesting a call if it "may" hold, only when it "must" hold.extract_callable_info
to generalize this suggestion to fn pointers, type parameters, and opaque types.Fixes #102852