Correct the padding on integer types for formatting

alexcrichton · alexcrichton · commit 1f6afa887b9f · 2013-08-12T23:18:51.000-07:00
diff --git a/src/libstd/fmt/mod.rs b/src/libstd/fmt/mod.rs
@@ -32,7 +32,7 @@ pub struct Formatter<'self> {
     /// Character used as 'fill' whenever there is alignment
     fill: char,
     /// Boolean indication of whether the output should be left-aligned
-    alignleft: bool,
+    align: parse::Alignment,
     /// Optionally specified integer width that the output should be
     width: Option<uint>,
     /// Optionally specified precision for numeric types
@@ -108,7 +108,7 @@ pub unsafe fn sprintf(fmt: &[rt::Piece], args: &[Argument]) -> ~str {
             precision: None,
             // FIXME(#8248): shouldn't need a transmute
             buf: cast::transmute(&output as &io::Writer),
-            alignleft: false,
+            align: parse::AlignUnknown,
             fill: ' ',
             args: args,
             curarg: args.iter(),
@@ -148,7 +148,7 @@ impl<'self> Formatter<'self> {
             rt::Argument(ref arg) => {
                 // Fill in the format parameters into the formatter
                 self.fill = arg.format.fill;
-                self.alignleft = arg.format.alignleft;
+                self.align = arg.format.align;
                 self.flags = arg.format.flags;
                 setcount(&mut self.width, &arg.format.width);
                 setcount(&mut self.precision, &arg.format.precision);
@@ -251,36 +251,47 @@ impl<'self> Formatter<'self> {
     /// TODO: dox
     pub fn pad_integral(&mut self, s: &[u8], alternate_prefix: &str,
                         positive: bool) {
-        use fmt::parse::{FlagAlternate, FlagSignPlus};
+        use fmt::parse::{FlagAlternate, FlagSignPlus, FlagSignAwareZeroPad};
 
         let mut actual_len = s.len();
         if self.flags & 1 << (FlagAlternate as uint) != 0 {
             actual_len += alternate_prefix.len();
         }
         if self.flags & 1 << (FlagSignPlus as uint) != 0 {
             actual_len += 1;
-        }
-        if !positive {
+        } else if !positive {
             actual_len += 1;
         }
 
-        let emit = |this: &mut Formatter| {
-            if this.flags & 1 << (FlagSignPlus as uint) != 0 && positive {
-                this.buf.write(['+' as u8]);
-            } else if !positive {
-                this.buf.write(['-' as u8]);
-            }
-            if this.flags & 1 << (FlagAlternate as uint) != 0 {
-                this.buf.write(alternate_prefix.as_bytes());
+        let mut signprinted = false;
+        let sign = |this: &mut Formatter| {
+            if !signprinted {
+                if this.flags & 1 << (FlagSignPlus as uint) != 0 && positive {
+                    this.buf.write(['+' as u8]);
+                } else if !positive {
+                    this.buf.write(['-' as u8]);
+                }
+                if this.flags & 1 << (FlagAlternate as uint) != 0 {
+                    this.buf.write(alternate_prefix.as_bytes());
+                }
+                signprinted = true;
             }
+        };
+
+        let emit = |this: &mut Formatter| {
+            sign(this);
             this.buf.write(s);
         };
 
         match self.width {
             None => { emit(self) }
             Some(min) if actual_len >= min => { emit(self) }
             Some(min) => {
-                do self.with_padding(min - actual_len) |me| {
+                if self.flags & 1 << (FlagSignAwareZeroPad as uint) != 0 {
+                    self.fill = '0';
+                    sign(self);
+                }
+                do self.with_padding(min - actual_len, parse::AlignRight) |me| {
                     emit(me);
                 }
             }
@@ -292,8 +303,8 @@ impl<'self> Formatter<'self> {
     /// recognized for generic strings are:
     ///
     /// * width - the minimum width of what to emit
-    /// * fill/alignleft - what to emit and where to emit it if the string
-    ///                    provided needs to be padded
+    /// * fill/align - what to emit and where to emit it if the string
+    ///                provided needs to be padded
     /// * precision - the maximum length to emit, the string is truncated if it
     ///               is longer than this length
     ///
@@ -336,23 +347,28 @@ impl<'self> Formatter<'self> {
             // If we're under both the maximum and the minimum width, then fill
             // up the minimum width with the specified string + some alignment.
             Some(width) => {
-                do self.with_padding(width - s.len()) |me| {
+                do self.with_padding(width - s.len(), parse::AlignLeft) |me| {
                     me.buf.write(s.as_bytes());
                 }
             }
         }
     }
 
-    fn with_padding(&mut self, padding: uint, f: &fn(&mut Formatter)) {
-        if self.alignleft {
+    fn with_padding(&mut self, padding: uint,
+                    default: parse::Alignment, f: &fn(&mut Formatter)) {
+        let align = match self.align {
+            parse::AlignUnknown => default,
+            parse::AlignLeft | parse::AlignRight => self.align
+        };
+        if align == parse::AlignLeft {
             f(self);
         }
         let mut fill = [0u8, ..4];
         let len = self.fill.encode_utf8(fill);
         for _ in range(0, padding) {
             self.buf.write(fill.slice_to(len));
         }
-        if !self.alignleft {
+        if align == parse::AlignRight {
             f(self);
         }
     }
@@ -427,7 +443,6 @@ macro_rules! upper_hex(($ty:ident, $into:ident) => {
         }
     }
 })
-
 // Not sure why, but this causes an "unresolved enum variant, struct or const"
 // when inlined into the above macro...
 #[doc(hidden)]
@@ -500,9 +515,10 @@ impl<T> Poly for T {
 //      time.
 impl<T> Pointer for *const T {
     fn fmt(t: &*const T, f: &mut Formatter) {
-        // XXX: formatting args
-        f.buf.write("0x".as_bytes());
-        LowerHex::fmt(&(*t as uint), f);
+        f.flags |= 1 << (parse::FlagAlternate as uint);
+        do ::uint::to_str_bytes(*t as uint, 16) |buf| {
+            f.pad_integral(buf, "0x", true);
+        }
     }
 }
 
diff --git a/src/libstd/fmt/parse.rs b/src/libstd/fmt/parse.rs
@@ -47,7 +47,7 @@ pub struct FormatSpec<'self> {
     /// Optionally specified character to fill alignment with
     fill: Option<char>,
     /// Optionally specified alignment
-    align: Option<Alignment>,
+    align: Alignment,
     /// Packed version of various flags provided
     flags: uint,
     /// The integer precision to use
@@ -68,7 +68,7 @@ pub enum Position<'self> {
 
 /// Enum of alignments which are supoprted.
 #[deriving(Eq)]
-pub enum Alignment { AlignLeft, AlignRight }
+pub enum Alignment { AlignLeft, AlignRight, AlignUnknown }
 
 /// Various flags which can be applied to format strings, the meaning of these
 /// flags is defined by the formatters themselves.
@@ -77,6 +77,7 @@ pub enum Flag {
     FlagSignPlus,
     FlagSignMinus,
     FlagAlternate,
+    FlagSignAwareZeroPad,
 }
 
 /// A count is used for the precision and width parameters of an integer, and
@@ -288,7 +289,7 @@ impl<'self> Parser<'self> {
     fn format(&mut self) -> FormatSpec<'self> {
         let mut spec = FormatSpec {
             fill: None,
-            align: None,
+            align: AlignUnknown,
             flags: 0,
             precision: CountImplied,
             width: CountImplied,
@@ -311,9 +312,9 @@ impl<'self> Parser<'self> {
         }
         // Alignment
         if self.consume('<') {
-            spec.align = Some(AlignLeft);
+            spec.align = AlignLeft;
         } else if self.consume('>') {
-            spec.align = Some(AlignRight);
+            spec.align = AlignRight;
         }
         // Sign flags
         if self.consume('+') {
@@ -326,6 +327,9 @@ impl<'self> Parser<'self> {
             spec.flags |= 1 << (FlagAlternate as uint);
         }
         // Width and precision
+        if self.consume('0') {
+            spec.flags |= 1 << (FlagSignAwareZeroPad as uint);
+        }
         spec.width = self.count();
         if self.consume('.') {
             if self.consume('*') {
@@ -597,7 +601,7 @@ mod tests {
     fn fmtdflt() -> FormatSpec<'static> {
         return FormatSpec {
             fill: None,
-            align: None,
+            align: AlignUnknown,
             flags: 0,
             precision: CountImplied,
             width: CountImplied,
@@ -656,7 +660,7 @@ mod tests {
             position: ArgumentIs(3),
             format: FormatSpec {
                 fill: None,
-                align: None,
+                align: AlignUnknown,
                 flags: 0,
                 precision: CountImplied,
                 width: CountImplied,
@@ -671,7 +675,7 @@ mod tests {
             position: ArgumentIs(3),
             format: FormatSpec {
                 fill: None,
-                align: Some(AlignRight),
+                align: AlignRight,
                 flags: 0,
                 precision: CountImplied,
                 width: CountImplied,
@@ -683,7 +687,7 @@ mod tests {
             position: ArgumentIs(3),
             format: FormatSpec {
                 fill: Some('0'),
-                align: Some(AlignLeft),
+                align: AlignLeft,
                 flags: 0,
                 precision: CountImplied,
                 width: CountImplied,
@@ -695,7 +699,7 @@ mod tests {
             position: ArgumentIs(3),
             format: FormatSpec {
                 fill: Some('*'),
-                align: Some(AlignLeft),
+                align: AlignLeft,
                 flags: 0,
                 precision: CountImplied,
                 width: CountImplied,
@@ -710,7 +714,7 @@ mod tests {
             position: ArgumentNext,
             format: FormatSpec {
                 fill: None,
-                align: None,
+                align: AlignUnknown,
                 flags: 0,
                 precision: CountImplied,
                 width: CountIs(10),
@@ -722,7 +726,7 @@ mod tests {
             position: ArgumentNext,
             format: FormatSpec {
                 fill: None,
-                align: None,
+                align: AlignUnknown,
                 flags: 0,
                 precision: CountIs(10),
                 width: CountIsParam(10),
@@ -734,7 +738,7 @@ mod tests {
             position: ArgumentNext,
             format: FormatSpec {
                 fill: None,
-                align: None,
+                align: AlignUnknown,
                 flags: 0,
                 precision: CountIsNextParam,
                 width: CountImplied,
@@ -746,7 +750,7 @@ mod tests {
             position: ArgumentNext,
             format: FormatSpec {
                 fill: None,
-                align: None,
+                align: AlignUnknown,
                 flags: 0,
                 precision: CountIsParam(10),
                 width: CountImplied,
@@ -761,7 +765,7 @@ mod tests {
             position: ArgumentNext,
             format: FormatSpec {
                 fill: None,
-                align: None,
+                align: AlignUnknown,
                 flags: (1 << FlagSignMinus as uint),
                 precision: CountImplied,
                 width: CountImplied,
@@ -773,7 +777,7 @@ mod tests {
             position: ArgumentNext,
             format: FormatSpec {
                 fill: None,
-                align: None,
+                align: AlignUnknown,
                 flags: (1 << FlagSignPlus as uint) | (1 << FlagAlternate as uint),
                 precision: CountImplied,
                 width: CountImplied,
@@ -788,7 +792,7 @@ mod tests {
             position: ArgumentIs(3),
             format: FormatSpec {
                 fill: None,
-                align: None,
+                align: AlignUnknown,
                 flags: 0,
                 precision: CountImplied,
                 width: CountImplied,
diff --git a/src/libstd/fmt/rt.rs b/src/libstd/fmt/rt.rs
@@ -36,7 +36,7 @@ pub struct Argument<'self> {
 
 pub struct FormatSpec {
     fill: char,
-    alignleft: bool,
+    align: parse::Alignment,
     flags: uint,
     precision: parse::Count,
     width: parse::Count,
diff --git a/src/libsyntax/ext/ifmt.rs b/src/libsyntax/ext/ifmt.rs
@@ -317,6 +317,10 @@ impl Context {
     /// Translate a `parse::Piece` to a static `rt::Piece`
     fn trans_piece(&mut self, piece: &parse::Piece) -> @ast::expr {
         let sp = self.fmtsp;
+        let parsepath = |s: &str| {
+            ~[self.ecx.ident_of("std"), self.ecx.ident_of("fmt"),
+              self.ecx.ident_of("parse"), self.ecx.ident_of(s)]
+        };
         let rtpath = |s: &str| {
             ~[self.ecx.ident_of("std"), self.ecx.ident_of("fmt"),
               self.ecx.ident_of("rt"), self.ecx.ident_of(s)]
@@ -482,20 +486,24 @@ impl Context {
                 let fill = self.ecx.expr_lit(sp, ast::lit_int(fill as i64,
                                                               ast::ty_char));
                 let align = match arg.format.align {
-                    None | Some(parse::AlignLeft) => {
-                        self.ecx.expr_bool(sp, true)
+                    parse::AlignLeft => {
+                        self.ecx.path_global(sp, parsepath("AlignLeft"))
+                    }
+                    parse::AlignRight => {
+                        self.ecx.path_global(sp, parsepath("AlignRight"))
                     }
-                    Some(parse::AlignRight) => {
-                        self.ecx.expr_bool(sp, false)
+                    parse::AlignUnknown => {
+                        self.ecx.path_global(sp, parsepath("AlignUnknown"))
                     }
                 };
+                let align = self.ecx.expr_path(align);
                 let flags = self.ecx.expr_uint(sp, arg.format.flags);
                 let prec = trans_count(arg.format.precision);
                 let width = trans_count(arg.format.width);
                 let path = self.ecx.path_global(sp, rtpath("FormatSpec"));
                 let fmt = self.ecx.expr_struct(sp, path, ~[
                     self.ecx.field_imm(sp, self.ecx.ident_of("fill"), fill),
-                    self.ecx.field_imm(sp, self.ecx.ident_of("alignleft"), align),
+                    self.ecx.field_imm(sp, self.ecx.ident_of("align"), align),
                     self.ecx.field_imm(sp, self.ecx.ident_of("flags"), flags),
                     self.ecx.field_imm(sp, self.ecx.ident_of("precision"), prec),
                     self.ecx.field_imm(sp, self.ecx.ident_of("width"), width),
diff --git a/src/test/run-pass/ifmt.rs b/src/test/run-pass/ifmt.rs
