Skip to content

Off-by-one in println! diagnostic #44954

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

Closed
steveklabnik opened this issue Oct 1, 2017 · 10 comments
Closed

Off-by-one in println! diagnostic #44954

steveklabnik opened this issue Oct 1, 2017 · 10 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics

Comments

@steveklabnik
Copy link
Member

This program:

fn main() {
    let name = "foo";
    
    println!("{} {}", name);
}

Gives this error:

error: invalid reference to argument `1` (there is 1 argument)

Both things having 1 here is confusing, it'd be nicer if we didn't compare a zero-indexed and a one-indexed number.

Additionally, a note could make this easier to understand overall too.

@steveklabnik steveklabnik 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 Oct 1, 2017
@estebank
Copy link
Contributor

estebank commented Oct 2, 2017

Mentoring instructions

Modify verify_arg_type to instead of outputting positional argument errors, to keep track of them (bubbling them up to the callee of verify_piece) so that the following cases are handled:

  • println!("{} {} {}", name);: All arguments are positional, estate the difference clearly:
    error: 3 positional arguments in format string, while only 1 was supplied
  • println!("{} {value} {}", name, value=2); There're named arguments in the string, keep current output:
    error: invalid reference to positional argument 3 (there are 3 arguments)
  • println!("{name} {value} {} {} {} {} {}", 0, name=1, value=2); There are multiple missing arguments, make a list and present only one diagnostic:
    error: invalid reference to positional arguments 4and5 (there are 3 arguments)

The following case are already handled in other parts of the code:

  • println!("{}", name, surname);

@estebank estebank added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics labels Oct 2, 2017
@TimNN TimNN added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Oct 3, 2017
@BrunoWilkinson
Copy link

Hi,

I was wondering if this PR is still ongoing? I've started learning Rust and I was wondering if you think I can tackle this, I'm very curious and driven to learn more about the language and also how programming languages are built.

Thank you for your feedback.

@estebank
Copy link
Contributor

estebank commented Oct 4, 2017

@BrunoWilkinson go ahead! People are available at https://gitter.im/rust-impl-period/WG-compiler-errors and on IRC for quick questions as well as on this issue for longer questions. Once you create a PR we can continue the conversation there. There are instructions to build the compiler, and if you want to use debug statements, you can use RUST_LOG=debug ./path/to/rustc file.rs and debug!() statements in the code will be printed out to stderr.

@BrunoWilkinson
Copy link

@estebank thank you, I'm gonna check all this out and give it a shot!

@bagedevimo
Copy link

Hey @BrunoWilkinson - are you still looking at this? If not, I'd like to have a go.

@BrunoWilkinson
Copy link

Hey @bagedevimo, currently no I couldn't find the time to tackle the issue, In parallel, I'm still learning Rust. But I would love to see the solution to this issue like I did spend some time on it.

@tommyip
Copy link
Contributor

tommyip commented Nov 3, 2017

Hey @bagedevimo are you working on this?

@bagedevimo
Copy link

bagedevimo commented Nov 3, 2017 via email

@tommyip
Copy link
Contributor

tommyip commented Nov 5, 2017

Currently

let name = "foo";
println!("{} {value} {}", name, value=2);

produces the output foo 2 2, is this the intended behavior and do we want to change it?

@estebank
Copy link
Contributor

estebank commented Nov 5, 2017

@tommyip I believe that is expected behavior, let's keep it and handle the other cases laid down in the mentoring instructions.

bors added a commit that referenced this issue Nov 11, 2017
Make positional argument error in format! clearer

r? @estebank

Fixes #44954
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-enhancement Category: An issue proposing an enhancement or a PR with one. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

No branches or pull requests

6 participants