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

format_args: allow omission of format string #12384

Closed

Conversation

seanmonstar
Copy link
Contributor

A default format string of Poly {:?} is generated to match the number
of passed arguments. This propagates to all fmt macros, such as println!,
debug!, format!, write!, etc.

Example: println!("{:?}", value); => println!(value);

fixes #12015

A default format string of Poly {:?} is generated to match the number
of passed arguments. This propagates to all fmt macros, such as println!,
debug!, format!, write!, etc.

Example: println!("{:?}", value); => println!(value);

fixes rust-lang#12015
@lilyball
Copy link
Contributor

What is the benefit of this, especially if it emits a note every time it's used?

If I see println!(foo) I'm going to expect that foo is a format string, and be surprised that println!() accepts dynamic format strings.

@seanmonstar
Copy link
Contributor Author

benefit is in #12015

You might expect foo to be a format string, hence the note to point out that no string literal was found, so we're using a generated one.

Other usage: JavaScript: console.log('%s %s', someVal, otherVal); and console.log(someVal, otherVal);

@lilyball
Copy link
Contributor

Why allow it at all, if it's going to unconditionally issue a note?

@seanmonstar
Copy link
Contributor Author

I don't understand what else you may be asking that isn't answered in previous comments and is in #12015.

@huonw
Copy link
Member

huonw commented Feb 19, 2014

#12015 makes no mention of the span_notes that are printed at compile time in this patch. With these notes, every single use of println!(value, value, value) causes a few lines of non-silenceable compiler output, which can drown out other warnings and errors.

ast::LitStr(ref s, _) => return Some(s.get().to_owned()),
_ => cx.span_note(l.span, note_msg)
},
_ => cx.span_note(expr.span, note_msg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and this one.

@bstrie
Copy link
Contributor

bstrie commented Feb 19, 2014

If I see println!(foo) I'm going to expect that foo is a format string, and be surprised that println!() accepts dynamic format strings.

Sadly, the inability of println! to accept non-string-literals will become quickly apparent to anyone using Rust for more than thirty minutes. There may be other arguments against this change, but the invocation is perfectly cromulent.

@seanmonstar
Copy link
Contributor Author

@huonw yea, I know the notes are there. I wrote them. Is the answer in #12384 (comment) not sufficient?

I may have the wrong understanding of span_note, as well.

@huonw
Copy link
Member

huonw commented Feb 19, 2014

@seanmonstar no, it doesn't address the compilation noise it generates, e.g.

fn foo(x: int) {
     println!(x); // note: format string literal missing, using default
     bar(x + 1);
}
fn bar(x: int) {
    println!(x); // note: format string literal missing, using default
    baz(x + 2);
}
fn baz(x: int) {
    println!(x); // note: format string literal missing, using default
    // ...
}
fn main() { foo(0) }

Everything single one of those println!s gets a note, so that's 9+ lines (including the source output and the ^~~~ span) of unsilenceable compiler output, always, making it harder to interpret any other output. And making it especially hard when there are some legitimate println!(x)'s and some others that are meant to be formatted but are accidentally of this form, e.g.

println!(some_variable); // ok, omitted format string on purpose

println!(&mut writer, "a {} b", 1); // oops, was meant to be `writeln!`

In any case, I think this shouldn't be done via println!. Something like debug_dump!(x, y, ..., z) seems like it would be better: it makes it clearer that it's for debugging only, and doesn't require the compiler to print things to stop people accidentally doing something wrong.

@alexcrichton
Copy link
Member

I personally think that println! should be used for one and only one thing, which is formatted printing. Debugging values belongs in a separate macro, not intertwined with the format strings.

@seanmonstar
Copy link
Contributor Author

I actually disagree on each point.

Coming from other languages like Java or JavaScript, something like println is sometimes use to output information to a user, such as for command line scripts, but almost seems more likely to just be quick debug lines. I pointed at past work with console.log as an example.

Using another macro to do the same thing, but to have a different name, seems less elegant. Java has seared into my mind that System.out.println() is a debug message more often than not.

As for the note at each usage, that seems quite fine to me. The extra noise can help you to remember to remove it after it's no longer needed. Plus, I'd expect an ability to eventually ignore note level messages from the compiler, such as rustc --level=warn mod.rs. Again, sort of how you can receive or ignore notices and warnings in Java, PHP, ActionScript, etc.

However, being the newcomer, I'll concede to not knowing what's best for Rust. Cheers.

@lilyball
Copy link
Contributor

I'd expect an ability to eventually ignore note level messages from the compiler, such as rustc --level=warn mod.rs. Again, sort of how you can receive or ignore notices and warnings in Java, PHP, ActionScript, etc.

That sounds like a terrible idea. If I'm seeing a note, it's because I'm doing something wrong. I should not be allowed to squelch those messages. Ignored warnings is a terrible problem in C/C++, and I do not want the same to happen to Rust.

@seanmonstar
Copy link
Contributor Author

That sounds like a terrible idea. If I'm seeing a note, it's because I'm doing something wrong. I should not be allowed to squelch those messages. Ignored warnings is a terrible problem in C/C++, and I do not want the same to happen to Rust.

Again, I must have misunderstood the severity of "note". "note" sounds potentially ignorable to me (it also outputs in green in terminal, which conveys "all good here, boss".)

@huonw
Copy link
Member

huonw commented Feb 19, 2014

I pointed at past work with console.log as an example.

console.log is not equivalent to println!: console.log is not designed for user-facing output at all; i.e. it's explicitly debugging, similar to our debug! macro or the debug_dump! I proposed above.

Plus, I'd expect an ability to eventually ignore note level messages from the compiler, such as rustc --level=warn mod.rs. Again, sort of how you can receive or ignore notices and warnings in Java, PHP, ActionScript, etc.

Silencing all notes hides legitimate misuse of println! (like in my second example here #12384 (comment)), as well as other helpful compiler output which come as notes. (i.e. the main error message comes as an error: ... with some helpful suggestions for what's the problem/how to fix it as notes.)

@seanmonstar
Copy link
Contributor Author

@huonw console.log is used for user-facing output, when writing nodejs command-line scripts. It's more common to use console.log than process.stdout.write(msg + '\n');

@lilyball
Copy link
Contributor

@seanmonstar

Again, I must have misunderstood the severity of "note". "note" sounds potentially ignorable to me (it also outputs in green in terminal, which conveys "all good here, boss".)

Any compiler output, beyond telling me that it's compiling a file, is a cause for concern. The compiler is not in the habit of printing messages along the lines of "good job, buddy!" Anything it says is because it thinks there's a problem. To that end, I do not want the ability to hide anything the compiler outputs. And given that, I do not want any API that will emit a note as a matter of normal usage.

Emitting a note due to a debugging function is ok, as it's a reminder that you're using a debugging function, but in that case I would expect to be using a different name altogether, such as dump!(), rather than a variant usage of println!().

@huonw
Copy link
Member

huonw commented Feb 19, 2014

Ah, ok (I've not used Node).

However, Node inherits its semantics of console.log from the browser, where it does have the property I say. The fact that the debugging printer (that doesn't even have a standardised output format) is abused as a normal printer in nodejs is mostly (but not completely) irrelevant. :)

@seanmonstar
Copy link
Contributor Author

Ok, I'll close this and try again with dump!.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022
…eykril

Generate variant: insert code in file with enum definition

Closes rust-lang#12372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a shorter macro for println!("{:?}", foo)
5 participants