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

Figure out what to do with {:x?} #165

Open
m-ou-se opened this issue Jan 17, 2023 · 5 comments
Open

Figure out what to do with {:x?} #165

m-ou-se opened this issue Jan 17, 2023 · 5 comments
Labels
help wanted Extra attention is needed T-libs-api

Comments

@m-ou-se m-ou-se added I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. T-libs-api labels Jan 17, 2023
@m-ou-se
Copy link
Member Author

m-ou-se commented Jan 31, 2023

We discussed this in a recent libs-api meeting.

The summary of the discussion is that we think {:x?} is currently in a weird state, and quite possibly shouldn't have happened. See this comment for details.

We suspect that the majority of use cases of {:x?} fall into one of two categories:

  1. Structs/enums with integer fields that should always be formatted in hexadecimal. (E.g. when using an usize to represent some kind of memory offset or checksum, etc.)
  2. For formatting lists/vecs/arrays of integers/bytes. (E.g. Vec<u8> as [f0, 9f, a6, 80].)

For 1, {:x?} works but isn't perfect, because it means you always have to use {:x?} rather than just {:?}. If more specific wrapper types around the integers (with a custom Debug impl) are not acceptable in these cases, perhaps we should consider extending the #[derive(Debug)] macro to allow e.g. #[debug(format = ..)] on individual fields, or something like that.

For 2, {:x?} works but isn't great either, because it has very limited flexibility. E.g. you can't enable the # flag to get 0x prefixes, without also enabling the # flag for multiline debug output. Some other language have more flexible formatting placeholders for collections/lists specifically. Perhaps we should have something like {:{:x}, } for "comma separated x-formatted elements" (syntax to be bikeshed of course).

This isn't to say that we guarantee to accept a proposal for either of these things­—any such proposal will still have to go through the usual process—but these do sound like reasonable things to have.

(If we end up having better solutions for the vast majority of use cases of {:x?}, we might even consider deprecating the use of {:x?} in format_args in the future.)

@m-ou-se m-ou-se removed the I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. label Jan 31, 2023
@clarfonthey
Copy link

One other possibility worth exploring would be to have methods on slices and clonable iterators which provide Debug impls which format things in different ways. For example, [impl LowerHex] could have a method that returns impl Debug that instead uses the LowerHex implementation.

@ojeda
Copy link

ojeda commented Feb 8, 2023

Related: rust-lang/rust#75766.

@pitaj
Copy link

pitaj commented Apr 9, 2023

I've been giving this a lot of thought over the last few days. At first, I really liked @dbdr's idea:

Could the # modifier apply only to the closest type, so that #x?, x#? and #x#? could be used to denote each case?

But the breaking change is unfortunate, and it doesn't compose very well with padding and width. The "nested format" syntax suggested by @m-ou-se is okay, but a little ugly. One alternative that combines the two approaches is putting the element formatting after the ?:

  • x? would become ?x
  • #x? would become #?#x
  • any existing options like width go before the ? if they should be applied to the whole value: 50?x
  • or after the ? if they should only apply to each element: #?50x
  • easy to migrate on an edition boundary

But going through these options just raises more questions for me:

  1. Do we want this to work with just Debug, or also with Display? (I think Debug only)
  2. Do we only care about numbers, or should we support strings and other forms of primitive values as well?
  3. Do we want to be able to specify the separator? (I think not worth it)
  4. What about arrays within other data structures or within arrays? (I think that element formatting should only apply to the deepest level)
  5. Should trying to use x? (or whatever new syntax we decide on) on [impl !LowerHex] produce a compile-time error/warning, clippy lint, a runtime panic, or fail silently?

API

This depends on the answers to the questions above, but I'll still share my thought process so far.

Originally, I thought of using a new trait for lists with an associated Item type that could be used to check whether the given formatting was available:

trait ListDebug {
    type Item;
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result;
}

impl<T> ListDebug for [T] {
    type Item = T;
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.debug_list().entries(self).finish()
    }
}

impl<T: ListDebug> Debug for T { /* delegate fmt */ }

But I quickly realized it would be difficult or impossible to make Item bounds checking work for nested data and such. So I dropped the associated type and considered adding more functions to Debug:

trait Debug {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result;
    
    /// `?x` formatting
    /// 
    /// Formats the value using the given formatter,
    /// formatting entries with `LowerHex` when possible.
    ///
    /// By default, this just calls `Debug::fmt`. Types
    /// implementing both `Debug` and `LowerHex` should
    /// override this function to call `LowerHex::fmt`.
    ///
    /// This gets used by the `entry` and `entries`
    /// functions of `DebugList`, `DebugMap`, and `DebugSet`
    /// when `f.debug_entry_flags().lower_hex() == true`.
    ///
    /// The `Formatter` passed to this function is constructed
    /// from the format specifier after the `?`.
    fn fmt_entry_lower_hex(&self, f: &mut fmt::Formatter<'_>) {
        Debug::fmt(&self, f);
    }
    
   //  ... and so on
}

But what is stated about DebugList using this on the entries would not work for nested data and may not even be possible.

What I ended up at was the following:

#[derive(Clone, Copy)]
#[non_exhaustive]
pub enum DebugNumberMode {
    LowerHex,
    UpperHex,
    Octal,
    Binary,
    LowerExp,
    UpperExp,
    None,
}

pub struct DebugNumber<'a>(fmt::Formatter<'a>);

impl<'a> DebugNumber<'a> {
    fn display(&mut self, n: &dyn fmt::Display) -> fmt::Result {
        n.fmt(&mut self.0)
    }

    fn lower_hex(&mut self, n: &dyn fmt::LowerHex) -> fmt::Result {
        n.fmt(&mut self.0)
    }
    
    // ...
}

impl<'a> Formatter<'a> {
    pub fn debug_number(&mut self) -> (DebugNumberMode, DebugNumber<'a>) {
        // return the entry formatting mode and new formatter based on the entry formatting flags
    }
}

// Usage

impl fmt::Debug for MyInt {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        let (mode, mut f) = debug_number(f);
        match mode {
            DebugNumberMode::LowerHex => f.lower_hex(self),
            // ...
            _ => f.display(self),
        }
    }
}

One variation on this is to require that someone who wants to use this API implement all of the traits and then only offer the following:

impl Formatter<'_> {
    pub fn debug_number(&mut self, n: &dyn Debug + Display + LowerHex + UpperHex ...) -> Result {
        // match on entry formatting mode
        // call the correct trait with a new formatter based on the entry formatting options
    }
}

If we want to support more than just numeric types, we could also provide debug_string:

impl Formatter<'_> {
    pub fn debug_string(&mut self, s: &dyn Debug) -> Result {
        // call `Debug::fmt` with a new formatter based on the entry formatting options
    }
}

@clarfonthey
Copy link

I personally have made traits similar to the ListDebug trait mentioned for my own projects, although it looked something more like:

pub trait DebugIter<'a> {
    type Iter: 'a + Iterator<Item = &'a dyn Debug>;
    fn debug_iter(&'a self) -> Self::Iter;
}

Since any such iterator can easily be composed into a final debug_list call.

Of course, the whole iterator construction isn't super necessary in most cases. I wonder if the right approach is really to just have methods on Iterator to make formatting something as various traits easier, and a way to just wrap an Iterator for a debug_list.

@dtolnay dtolnay added the help wanted Extra attention is needed label Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed T-libs-api
Projects
None yet
Development

No branches or pull requests

5 participants