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

Nonsense suggestion for concat!(x) #52347

Closed
ollie27 opened this issue Jul 13, 2018 · 4 comments
Closed

Nonsense suggestion for concat!(x) #52347

ollie27 opened this issue Jul 13, 2018 · 4 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug.

Comments

@ollie27
Copy link
Member

ollie27 commented Jul 13, 2018

The error for concat!(x) is:

error: expected a literal
 --> src/main.rs:2:13
  |
2 |     concat!(x);
  |             ^
help: you might be missing a string literal to format with
  |
2 |     concat!("{}", x);
  |             ^^^^^^^

That suggestion will clearly not work and gets worse when there are more arguments:

error: expected a literal
 --> src/main.rs:2:13
  |
2 |     concat!(x, y, z);
  |             ^
help: you might be missing a string literal to format with
  |
2 |     concat!("{}", x, y, z);
  |             ^^^^^^^

error: expected a literal
 --> src/main.rs:2:16
  |
2 |     concat!(x, y, z);
  |                ^
help: you might be missing a string literal to format with
  |
2 |     concat!(x, "{}", y, z);
  |                ^^^^^^^

error: expected a literal
 --> src/main.rs:2:19
  |
2 |     concat!(x, y, z);
  |                   ^
help: you might be missing a string literal to format with
  |
2 |     concat!(x, y, "{}", z);
  |                   ^^^^^^^

This suggestion was added by #51614. cc. @csmoe, @estebank

@oli-obk oli-obk added A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. labels Jul 13, 2018
@csmoe csmoe self-assigned this Jul 13, 2018
@estebank
Copy link
Contributor

@csmoe, we might get away with println calling a specialized concat where the builtin is just one method with different diagnostics depending on the callee. Just a thought though, you'll likely find a better solution :)

@estebank
Copy link
Contributor

It'd be ideal if we could also keep the expected a literal messages to a minimum, by emitting only one diagnostic with a MultiSpan primary span. (In the last example, instead of three errors, only one pointing at all three arguments.)

@csmoe
Copy link
Member

csmoe commented Jul 14, 2018

@estebank I suspect this issue is more complex than I thought, here is a testcase:

fn main() {
    concat!(a);     // ERROR: ("{}", a)
    println!(a);    // FINE: ("{}", a)
}

playground
Since we are working with concat.rs here, when only ONE arg is passed into them, the emitted "{}", foo message ONLY helps println!, not concat!. And we cannot distinguish between println! and concat! here(only-one-arg-passed-in method fails), so I think such kind of help-message is blocked by the mixed literal and format string issue(print!s accept literal and format string, but concat! ONLY accepts literal), #51614 should be retrieved.


The only way I can think of is splitting concat! from print!s, and rewrite them into compiler-builtin macros(print.rs) like conat.rs for concat!.

@ollie27
Copy link
Member Author

ollie27 commented Jul 14, 2018

What I think should be done is for the suggestion to be moved to format_args! where it should always make sense. It would then work for macros like write!, print!, format! and format_args! itself. Then to get it working for the *ln! macros fixing #30143 would be enough. Maybe the prospect of better diagnostics would persuade people that it's worth fixing.

bors added a commit that referenced this issue Jul 15, 2018
Improve suggestion for missing fmt str in println

Avoid using `concat!(fmt, "\n")` to improve the diagnostics being
emitted when the first `println!()` argument isn't a formatting string
literal.

Fix #52347.
bors added a commit that referenced this issue Jul 15, 2018
Improve suggestion for missing fmt str in println

Avoid using `concat!(fmt, "\n")` to improve the diagnostics being
emitted when the first `println!()` argument isn't a formatting string
literal.

Fix #52347.
bors added a commit that referenced this issue Jul 20, 2018
Improve suggestion for missing fmt str in println

Avoid using `concat!(fmt, "\n")` to improve the diagnostics being
emitted when the first `println!()` argument isn't a formatting string
literal.

Fix #52347.
bors added a commit that referenced this issue Jul 22, 2018
Improve suggestion for missing fmt str in println

Avoid using `concat!(fmt, "\n")` to improve the diagnostics being
emitted when the first `println!()` argument isn't a formatting string
literal.

Fix #52347.
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.
Projects
None yet
Development

No branches or pull requests

4 participants