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

Make formatting available without alloc #1126

Closed
wants to merge 5 commits into from

Conversation

pitdicker
Copy link
Collaborator

@pitdicker pitdicker commented Jun 5, 2023

It is possible to make the formatting methods available without the alloc feature with relatively little changes.

  1. Format to fmt::Formatter instead of String. This means replacing push_str() with write_str()?, and replacing push() with write_chr()?.
  2. When creating a DelayedFormat the timezone name is rendered to a String. I have replaced this with a fixed-size buffer of 63 bytes. It was slightly tricky to get the Display method of Offset to format into a byte buffer, that is what TzName is for.
  3. Display can take some tricky formatting specifiers to justify/pad/truncate our formatting result. This is very difficult to do without rendering the intermediate result to a String. As solution I choose to render to an intermediate string in such a case in Display::fmt, and to ignore the specifiers in no_std.

Part 2 and 3 seem like a small price to pay for making all these methods available to no_std targets.

Previously DelayedFormat gave the impression it would work without allocations (also in the documentation), because 'why else would we have this type'? But the first thing it did was create a temporary string to write into 😆.

@pitdicker pitdicker force-pushed the format_no_alloc branch 3 times, most recently from 0e7d267 to 5690734 Compare June 5, 2023 07:30
@pitdicker
Copy link
Collaborator Author

Rustfmt alternates with how it wants to format the large match in format_inner. It also did this in #1082. I have included the reformat as a separate commit.

@djc
Copy link
Member

djc commented Jun 5, 2023

Let's discuss in #1127 and see whether it still makes sense to also do this.

@pitdicker pitdicker force-pushed the format_no_alloc branch 2 times, most recently from f4b9c50 to 773d6c4 Compare June 8, 2023 16:37
@pitdicker
Copy link
Collaborator Author

When we add a new formatting API I think we can do better than writing the timezone name to a fixed-size string on every use.

@pitdicker pitdicker closed this Jun 29, 2023
@pitdicker pitdicker deleted the format_no_alloc branch July 21, 2023 11:54
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.

2 participants