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

Diagnostic misclassifies a complex expression as a unit enum variant when flagging an attempt to call a non-function #99240

Closed
fmease opened this issue Jul 14, 2022 · 1 comment · Fixed by #99350
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@fmease
Copy link
Member

fmease commented Jul 14, 2022

Given the following code:

fn fmt(it: &(std::cell::Cell<Option<impl FnOnce()>>,)) {
    (it.0.take())()
}

The current output is:

error[E0618]: expected function, found enum variant `(it.0.take())`
  --> src/lib.rs:22:5
   |
22 |     (it.0.take())()
   |     ^^^^^^^^^^^^^--
   |     |
   |     call expression requires function
   |
help: `(it.0.take())` is a unit variant, you need to write it without the parentheses
   |
22 -     (it.0.take())()
22 +     (it.0.take())
   |

  • It classifies the complex expression (it.0.take()) as a (unit) enum variant in both the message and the note which is obviously incorrect.
  • It does not mention anywhere that the expression is of type Option<_> which would've been really helpful. Looking at the current output, it's really hard to see why the expression is not callable.

( As an aside, one actual fix for the given code is the following: it.0.take().unwrap()() )

Link to the playground containing the reproducer, an even smaller reproducer and the original code for context.
I decided to select the slightly larger reproducer (containing the 1-tuple) for this issue instead of the smaller one from the playground (which can probably still be shrunk further) to really highlight the complex expression: (it.0.take()) over just it.take().

Reproduces on stable and on nightly.

@rustbot label C-bug D-incorrect D-confusing

@fmease fmease added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 14, 2022
@rustbot rustbot added C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels Jul 14, 2022
@fmease
Copy link
Member Author

fmease commented Jul 14, 2022

This check is too lax:

if let ty::Adt(adt_def, ..) = t
&& adt_def.is_enum()
&& let hir::ExprKind::Call(expr, _) = call_expr.kind

@compiler-errors compiler-errors self-assigned this Jul 16, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jul 17, 2022
…-dead

Be more precise when suggesting removal of parens on unit ctor

* Fixes rust-lang#99240 by only suggesting to remove parens on path exprs, not arbitrary expressions with enum type
* Generalizes by suggesting removal of parens on unit struct, too, because why not?
@bors bors closed this as completed in cc35c78 Jul 18, 2022
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 C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. 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.

3 participants