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 Format trait dyn compatible #926

Closed
wants to merge 5 commits into from

Conversation

jordens
Copy link

@jordens jordens commented Jan 29, 2025

  • Format::_format_tag() -> Format::_format_tag(&self)
  • change the wire format for empty slices a usize(0)
  • change the wire format for empty arrays to be empty

closes: #861

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
* `Format::_format_tag()` -> `Format::_format_tag(&self)`
* change the wire format for empty slices and arrays to be just a `usize(0)`
@jonathanpallant
Copy link
Contributor

Someone can now write:

let slice: &[&dyn defmt::Format] = &[&1.0, &2];
defmt::info!("{}", slice);

But I see:

INFO [1.0, 1e-323]

I think this is because the bits for 2i32 have been interpreted as an f32.

@jordens
Copy link
Author

jordens commented Jan 29, 2025

Yeah. That's problematic.

@jordens
Copy link
Author

jordens commented Jan 29, 2025

After a lengthy discussion in the chat:

There is a way to correctly support [&dyn Format] (and [Box<dyn Format>] etc) along the lines of
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=08d81d35efe5c2cbe41b614f409bc72c

But it requires restricting the blanket impl Format for & (and &mut, Box etc) to only sized types. Instead one would need to explicitly add impls for such containers of known unsized types: dyn Format, [T], and str (maybe others?).

It's unclear to me whether that's acceptable. If not, then this PR can be closed.

@jonathanpallant
Copy link
Contributor

That would count as a breaking change for anyone currently relying on that impl, and so I don't think we can do that. Thank you for digging through the details though and putting together this PR. It is really appreciated.

@jordens
Copy link
Author

jordens commented Jan 31, 2025

That leaves the change to the wire format omitting the tag for empty slices and arrays. What are the prospects there?

@jordens
Copy link
Author

jordens commented Jan 31, 2025

#929

@jordens jordens closed this Jan 31, 2025
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.

Object safe Format
2 participants