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

Improve error output for "expected value, found type parameter" #124634

Open
mhelsley opened this issue May 2, 2024 · 1 comment
Open

Improve error output for "expected value, found type parameter" #124634

mhelsley opened this issue May 2, 2024 · 1 comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. E-help-wanted Call for participation: Help is requested to fix this issue. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mhelsley
Copy link

mhelsley commented May 2, 2024

Code

pub fn delayed_insert<'a, T, C: Clone + Component>
        (ecommands: &'a mut EntityCommands<'a>,
         delay: Duration, c: C)
        -> &'a mut EntityCommands<'a> {
    ecommands.insert(DelayedInsert<T, C>::new(delay, c))
}

Current output

error[E0423]: expected value, found type parameter `T`
  --> src/delay.rs:22:36
   |
18 | pub fn delayed_insert<'a, T, C: Clone + Component>
   |                           - found this type parameter
...
22 |     ecommands.insert(DelayedInsert<T, C>::new(delay, c))
   |                                    ^ help: a local variable with a similar name exists: `c`

Desired output

error[E0423]: expected value, found type parameter `T`
  --> src/delay.rs:22:36
   |
18 | pub fn delayed_insert<'a, T, C: Clone + Component>
   |                           - found this type parameter
...
22 |     ecommands.insert(DelayedInsert<T, C>::new(delay, c))
   |                                    ^ help: a local variable with a similar name exists: `c`
   |                                    ^ alternate help: Substituting ::< for < may fix this

Rationale and extra context

In some cases rustc correctly suggests that the turbofish syntax is required.

Here, however, it looks like user intent is ambiguous and the compiler expects a comparison operator (which expects a value on the right of the < operator) but the error help doesn't offer the "correct" suggestion. So it may be helpful to offer an aldditional suggestion whenever the output is "expected value, found type parameter": use turbofish syntax.

I suspect this shouldn't require gathering or printing any more information than the compiler already has at-hand in this context -- a static, alternative, suggestion is likely all that'd be needed.

Other cases

No response

Rust Version

rustc 1.77.2 (25ef9e3d8 2024-04-09)
binary: rustc
commit-hash: 25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04
commit-date: 2024-04-09
host: x86_64-unknown-linux-gnu
release: 1.77.2
LLVM version: 17.0.6

Anything else?

Might be as easy to close this issue as looking for that string in the rustc source and modifying an existing output string.

@mhelsley mhelsley 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 May 2, 2024
@fmease fmease added the D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. label May 2, 2024
@workingjubilee workingjubilee added E-help-wanted Call for participation: Help is requested to fix this issue. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels May 6, 2024
@workingjubilee
Copy link
Contributor

It's right here:

let mut expected = source.descr_expected();
let path_str = Segment::names_to_string(path);
let item_str = path.last().unwrap().ident;
if let Some(res) = res {
BaseError {
msg: format!("expected {}, found {} `{}`", expected, res.descr(), path_str),
fallback_label: format!("not a {expected}"),
span,
span_label: match res {
Res::Def(DefKind::TyParam, def_id) => {
Some((self.r.def_span(def_id), "found this type parameter"))
}
_ => None,
},
could_be_expr: match res {
Res::Def(DefKind::Fn, _) => {
// Verify whether this is a fn call or an Fn used as a type.
self.r
.tcx
.sess
.source_map()
.span_to_snippet(span)
.is_ok_and(|snippet| snippet.ends_with(')'))
}
Res::Def(
DefKind::Ctor(..) | DefKind::AssocFn | DefKind::Const | DefKind::AssocConst,
_,
)
| Res::SelfCtor(_)
| Res::PrimTy(_)
| Res::Local(_) => true,
_ => false,
},
suggestion: None,
module: None,
}

At that point we have access to the resolver which has access to... most of everything, yes. We can indeed use span_to_snippet again and fix it there, though this is essentially an incorrect parsing condition.

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-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. E-help-wanted Call for participation: Help is requested to fix this issue. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants