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

'attempt to use non-constant in a constant' should point to the constant #79919

Closed
jyn514 opened this issue Dec 10, 2020 · 2 comments · Fixed by #80012
Closed

'attempt to use non-constant in a constant' should point to the constant #79919

jyn514 opened this issue Dec 10, 2020 · 2 comments · Fixed by #80012
Assignees
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Dec 10, 2020

(playground)

extern crate reqwest; // 0.10.9

fn main() {
    let s = String::from("http://example.com");
    const response = reqwest::get(&s);
}
error[E0435]: attempt to use a non-constant value in a constant
 --> src/main.rs:5:36
  |
5 |     const response = reqwest::get(&s);
  |                                    ^ non-constant value

I had const response = self.request(...) when I should have had let response = self.request(...) ... too much Javascript.
I don't think I equated "a constant" with const foo = ... in my head, because I'm used to doing const foo = bar(...) in Javascript all the time

It would be nice for the error to point to response as the constant:

5 |     const response = reqwest::get(&s);
  |           ^^^^^^^^ constant        ^ non-constant value
@jyn514 jyn514 added C-enhancement Category: An issue proposing an enhancement or a PR with one. 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. A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) D-papercut Diagnostics: An error or lint that needs small tweaks. labels Dec 10, 2020
@sasurau4
Copy link
Contributor

I'll try it
@rustbot claim

@estebank
Copy link
Contributor

My position,from my comment in the PR:

The code changes themselves look reasonable. The ui change I'm ambivalent about. I see in the original report why this might be a problem for people coming from JS, but I don't know that this change will be a solution. I can see other possibilities, like detecting that the const is in an item body (in other words, inside an fn) and then potentially suggest let instead of const. For other cases (like the one mentioned in the help for E0435, we could emit a label pointing at the definition of the non-const binding and suggesting turning that into a const (if the value can be const). If none of these cases fit, I think that the current output is reasonable enough, but could be expanded by saying "this binding can't be const, consider using a literal of the appropriate type" or something along those lines.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jan 7, 2021
…ntifier-E0435, r=petrochenkov

Add pointing const identifier when emitting E0435

Fix rust-lang#79919
@bors bors closed this as completed in bb229b8 Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-papercut Diagnostics: An error or lint that needs small tweaks. 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