Skip to content

Finish the polish on format! #2249

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
catamorphism opened this issue Apr 19, 2012 · 8 comments
Closed

Finish the polish on format! #2249

catamorphism opened this issue Apr 19, 2012 · 8 comments
Labels
A-syntaxext Area: Syntax extensions C-cleanup Category: PRs that clean code up or issues documenting cleanup. metabug Issues about issues themselves ("bugs about bugs")

Comments

@catamorphism
Copy link
Contributor

As per FIXMEs in syntax::ext::fmt, refactor nested functions inside pieces_to_expr, and also move validation code from make_new_conv into something in core::extfmt.

@catamorphism
Copy link
Contributor Author

Also: decide how important it is to be printf-compatible and whether parse_type should support two different "signed" specifiers.

bors added a commit that referenced this issue Mar 22, 2013
This is a minor step towards #3571, although I'm sure there's still more work to be done. Previously, `fmt!` collected a bunch of strings in a vector and then called `str::concat`. This changes the behavior by maintaining only one buffer and appending directly into that buffer. This avoids doubly-allocating memory, and it has the added bonus of reducing some allocations in `core::unstable::extfmt`

One of the unfortunate side effects of this is that the `rt` module in `extfmt.rs` had to be duplicated to avoid `stage0` errors. Dealing with the change in conversion functions may require a bit of a dance when a snapshot happens, but I think it's doable.

If the second speedup commit isn't deemed necessary, I got about a 15% speedup with just the first patch which doesn't require any modification of `extfmt.rs`, so no snapshot weirdness.

Here's some other things I ran into when looking at `fmt!`:
* I don't think that #2249 is relevant any more except for maybe removing one of `%i` or `%d`
* I'm not sure what was in mind for using traits with #3571, but I thought that formatters like `%u` could invoke the `to_uint()` method on the `NumCast` trait, but I ran into some problems like those in #5462

I'm having trouble thinking of other wins for `fmt!`, but if there's some suggestions I'd be more than willing to look into if they'd work out or not.
@pcwalton
Copy link
Contributor

I plan to rewrite fmt as part of the new I/O design.

@msullivan
Copy link
Contributor

I am renaming this bug and promoting it to the metabug for fmt! redesign. I am moving it up to milestone 2, because we may wish to change fmt in non backwards compatible ways.

Linking all of the fmt! issues I have found:

Major rewrite/redesign:

Feature requests:

Bugs/misfeatures:

@huonw
Copy link
Member

huonw commented Jul 27, 2013

Linking to the Lib fmt wiki page.

@alexcrichton
Copy link
Member

Changing name from "Rewrite fmt!" because that's mostly been done and this is now polish.

@huonw
Copy link
Member

huonw commented Aug 30, 2013

(I updated @msullivan's comment to a task list, and it seems to work.)

@catamorphism
Copy link
Contributor Author

De-milestoning. Enough of the work is done for now.

@brson
Copy link
Contributor

brson commented Jan 13, 2015

Obsolete.

@brson brson closed this as completed Jan 13, 2015
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 22, 2022
Format tests with rustfmt (151-200 of 300)

Extracted from rust-lang#2097.

This PR is still only doing the easy cases with no comments involved.

In the next PRs after this, I'll start grouping by common comment patterns, e.g. all the cases resembling rust-lang/miri#2097 (comment) together in one PR.
BoxyUwU pushed a commit to BoxyUwU/rust that referenced this issue Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-syntaxext Area: Syntax extensions C-cleanup Category: PRs that clean code up or issues documenting cleanup. metabug Issues about issues themselves ("bugs about bugs")
Projects
None yet
Development

No branches or pull requests

6 participants