Rollup merge of #106806 - m-ou-se:format-args-flags, r=oli-obk

JohnTitor · web-flow · commit 1163279c3a54 · 2023-01-28T00:23:11.000+09:00
Replace format flags u32 by enums and bools. This gets rid of the `flags: u32` field where each bit has a special meaning, and replaces it by simple enums and booleans. Part of #99012
diff --git a/compiler/rustc_ast/src/format.rs b/compiler/rustc_ast/src/format.rs
@@ -227,8 +227,30 @@ pub struct FormatOptions {
     pub alignment: Option<FormatAlignment>,
     /// The fill character. E.g. the `.` in `{:.>10}`.
     pub fill: Option<char>,
-    /// The `+`, `-`, `0`, `#`, `x?` and `X?` flags.
-    pub flags: u32,
+    /// The `+` or `-` flag.
+    pub sign: Option<FormatSign>,
+    /// The `#` flag.
+    pub alternate: bool,
+    /// The `0` flag. E.g. the `0` in `{:02x}`.
+    pub zero_pad: bool,
+    /// The `x` or `X` flag (for `Debug` only). E.g. the `x` in `{:x?}`.
+    pub debug_hex: Option<FormatDebugHex>,
+}
+
+#[derive(Copy, Clone, Encodable, Decodable, Debug, PartialEq, Eq)]
+pub enum FormatSign {
+    /// The `+` flag.
+    Plus,
+    /// The `-` flag.
+    Minus,
+}
+
+#[derive(Copy, Clone, Encodable, Decodable, Debug, PartialEq, Eq)]
+pub enum FormatDebugHex {
+    /// The `x` flag in `{:x?}`.
+    Lower,
+    /// The `X` flag in `{:X?}`.
+    Upper,
 }
 
 #[derive(Copy, Clone, Encodable, Decodable, Debug, PartialEq, Eq)]
diff --git a/compiler/rustc_ast_lowering/src/format.rs b/compiler/rustc_ast_lowering/src/format.rs
@@ -137,26 +137,43 @@ fn make_format_spec<'hir>(
         }
         Err(_) => ctx.expr(sp, hir::ExprKind::Err),
     };
-    let fill = ctx.expr_char(sp, placeholder.format_options.fill.unwrap_or(' '));
+    let &FormatOptions {
+        ref width,
+        ref precision,
+        alignment,
+        fill,
+        sign,
+        alternate,
+        zero_pad,
+        debug_hex,
+    } = &placeholder.format_options;
+    let fill = ctx.expr_char(sp, fill.unwrap_or(' '));
     let align = ctx.expr_lang_item_type_relative(
         sp,
         hir::LangItem::FormatAlignment,
-        match placeholder.format_options.alignment {
+        match alignment {
             Some(FormatAlignment::Left) => sym::Left,
             Some(FormatAlignment::Right) => sym::Right,
             Some(FormatAlignment::Center) => sym::Center,
             None => sym::Unknown,
         },
     );
-    let flags = ctx.expr_u32(sp, placeholder.format_options.flags);
-    let prec = make_count(ctx, sp, &placeholder.format_options.precision, argmap);
-    let width = make_count(ctx, sp, &placeholder.format_options.width, argmap);
+    // This needs to match `FlagV1` in library/core/src/fmt/mod.rs.
+    let flags: u32 = ((sign == Some(FormatSign::Plus)) as u32)
+        | ((sign == Some(FormatSign::Minus)) as u32) << 1
+        | (alternate as u32) << 2
+        | (zero_pad as u32) << 3
+        | ((debug_hex == Some(FormatDebugHex::Lower)) as u32) << 4
+        | ((debug_hex == Some(FormatDebugHex::Upper)) as u32) << 5;
+    let flags = ctx.expr_u32(sp, flags);
+    let precision = make_count(ctx, sp, &precision, argmap);
+    let width = make_count(ctx, sp, &width, argmap);
     let format_placeholder_new = ctx.arena.alloc(ctx.expr_lang_item_type_relative(
         sp,
         hir::LangItem::FormatPlaceholder,
         sym::new,
     ));
-    let args = ctx.arena.alloc_from_iter([position, fill, align, flags, prec, width]);
+    let args = ctx.arena.alloc_from_iter([position, fill, align, flags, precision, width]);
     ctx.expr_call_mut(sp, format_placeholder_new, args)
 }
 
diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
@@ -6,7 +6,10 @@ use rustc_ast::token;
 use rustc_ast::util::literal::escape_byte_str_symbol;
 use rustc_ast::util::parser::{self, AssocOp, Fixity};
 use rustc_ast::{self as ast, BlockCheckMode};
-use rustc_ast::{FormatAlignment, FormatArgPosition, FormatArgsPiece, FormatCount, FormatTrait};
+use rustc_ast::{
+    FormatAlignment, FormatArgPosition, FormatArgsPiece, FormatCount, FormatDebugHex, FormatSign,
+    FormatTrait,
+};
 use std::fmt::Write;
 
 impl<'a> State<'a> {
@@ -675,17 +678,15 @@ pub fn reconstruct_format_args_template_string(pieces: &[FormatArgsPiece]) -> St
                     Some(FormatAlignment::Center) => template.push_str("^"),
                     None => {}
                 }
-                let flags = p.format_options.flags;
-                if flags >> (rustc_parse_format::FlagSignPlus as usize) & 1 != 0 {
-                    template.push('+');
-                }
-                if flags >> (rustc_parse_format::FlagSignMinus as usize) & 1 != 0 {
-                    template.push('-');
+                match p.format_options.sign {
+                    Some(FormatSign::Plus) => template.push('+'),
+                    Some(FormatSign::Minus) => template.push('-'),
+                    None => {}
                 }
-                if flags >> (rustc_parse_format::FlagAlternate as usize) & 1 != 0 {
+                if p.format_options.alternate {
                     template.push('#');
                 }
-                if flags >> (rustc_parse_format::FlagSignAwareZeroPad as usize) & 1 != 0 {
+                if p.format_options.zero_pad {
                     template.push('0');
                 }
                 if let Some(width) = &p.format_options.width {
@@ -709,11 +710,10 @@ pub fn reconstruct_format_args_template_string(pieces: &[FormatArgsPiece]) -> St
                         }
                     }
                 }
-                if flags >> (rustc_parse_format::FlagDebugLowerHex as usize) & 1 != 0 {
-                    template.push('x');
-                }
-                if flags >> (rustc_parse_format::FlagDebugUpperHex as usize) & 1 != 0 {
-                    template.push('X');
+                match p.format_options.debug_hex {
+                    Some(FormatDebugHex::Lower) => template.push('x'),
+                    Some(FormatDebugHex::Upper) => template.push('X'),
+                    None => {}
                 }
                 template.push_str(match p.format_trait {
                     FormatTrait::Display => "",
diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs
@@ -4,7 +4,7 @@ use rustc_ast::tokenstream::TokenStream;
 use rustc_ast::{
     Expr, ExprKind, FormatAlignment, FormatArgPosition, FormatArgPositionKind, FormatArgs,
     FormatArgsPiece, FormatArgument, FormatArgumentKind, FormatArguments, FormatCount,
-    FormatOptions, FormatPlaceholder, FormatTrait,
+    FormatDebugHex, FormatOptions, FormatPlaceholder, FormatSign, FormatTrait,
 };
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::{pluralize, Applicability, MultiSpan, PResult};
@@ -435,7 +435,16 @@ pub fn make_format_args(
                     format_options: FormatOptions {
                         fill: format.fill,
                         alignment,
-                        flags: format.flags,
+                        sign: format.sign.map(|s| match s {
+                            parse::Sign::Plus => FormatSign::Plus,
+                            parse::Sign::Minus => FormatSign::Minus,
+                        }),
+                        alternate: format.alternate,
+                        zero_pad: format.zero_pad,
+                        debug_hex: format.debug_hex.map(|s| match s {
+                            parse::DebugHex::Lower => FormatDebugHex::Lower,
+                            parse::DebugHex::Upper => FormatDebugHex::Upper,
+                        }),
                         precision,
                         width,
                     },
diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs
@@ -16,7 +16,6 @@
 
 pub use Alignment::*;
 pub use Count::*;
-pub use Flag::*;
 pub use Piece::*;
 pub use Position::*;
 
@@ -111,8 +110,14 @@ pub struct FormatSpec<'a> {
     pub fill: Option<char>,
     /// Optionally specified alignment.
     pub align: Alignment,
-    /// Packed version of various flags provided.
-    pub flags: u32,
+    /// The `+` or `-` flag.
+    pub sign: Option<Sign>,
+    /// The `#` flag.
+    pub alternate: bool,
+    /// The `0` flag.
+    pub zero_pad: bool,
+    /// The `x` or `X` flag. (Only for `Debug`.)
+    pub debug_hex: Option<DebugHex>,
     /// The integer precision to use.
     pub precision: Count<'a>,
     /// The span of the precision formatting flag (for diagnostics).
@@ -162,24 +167,22 @@ pub enum Alignment {
     AlignUnknown,
 }
 
-/// Various flags which can be applied to format strings. The meaning of these
-/// flags is defined by the formatters themselves.
+/// Enum for the sign flags.
 #[derive(Copy, Clone, Debug, PartialEq)]
-pub enum Flag {
-    /// A `+` will be used to denote positive numbers.
-    FlagSignPlus,
-    /// A `-` will be used to denote negative numbers. This is the default.
-    FlagSignMinus,
-    /// An alternate form will be used for the value. In the case of numbers,
-    /// this means that the number will be prefixed with the supplied string.
-    FlagAlternate,
-    /// For numbers, this means that the number will be padded with zeroes,
-    /// and the sign (`+` or `-`) will precede them.
-    FlagSignAwareZeroPad,
-    /// For Debug / `?`, format integers in lower-case hexadecimal.
-    FlagDebugLowerHex,
-    /// For Debug / `?`, format integers in upper-case hexadecimal.
-    FlagDebugUpperHex,
+pub enum Sign {
+    /// The `+` flag.
+    Plus,
+    /// The `-` flag.
+    Minus,
+}
+
+/// Enum for the debug hex flags.
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub enum DebugHex {
+    /// The `x` flag in `{:x?}`.
+    Lower,
+    /// The `X` flag in `{:X?}`.
+    Upper,
 }
 
 /// A count is used for the precision and width parameters of an integer, and
@@ -597,7 +600,10 @@ impl<'a> Parser<'a> {
         let mut spec = FormatSpec {
             fill: None,
             align: AlignUnknown,
-            flags: 0,
+            sign: None,
+            alternate: false,
+            zero_pad: false,
+            debug_hex: None,
             precision: CountImplied,
             precision_span: None,
             width: CountImplied,
@@ -626,13 +632,13 @@ impl<'a> Parser<'a> {
         }
         // Sign flags
         if self.consume('+') {
-            spec.flags |= 1 << (FlagSignPlus as u32);
+            spec.sign = Some(Sign::Plus);
         } else if self.consume('-') {
-            spec.flags |= 1 << (FlagSignMinus as u32);
+            spec.sign = Some(Sign::Minus);
         }
         // Alternate marker
         if self.consume('#') {
-            spec.flags |= 1 << (FlagAlternate as u32);
+            spec.alternate = true;
         }
         // Width and precision
         let mut havewidth = false;
@@ -647,7 +653,7 @@ impl<'a> Parser<'a> {
                 spec.width_span = Some(self.span(end - 1, end + 1));
                 havewidth = true;
             } else {
-                spec.flags |= 1 << (FlagSignAwareZeroPad as u32);
+                spec.zero_pad = true;
             }
         }
 
@@ -678,14 +684,14 @@ impl<'a> Parser<'a> {
         // Optional radix followed by the actual format specifier
         if self.consume('x') {
             if self.consume('?') {
-                spec.flags |= 1 << (FlagDebugLowerHex as u32);
+                spec.debug_hex = Some(DebugHex::Lower);
                 spec.ty = "?";
             } else {
                 spec.ty = "x";
             }
         } else if self.consume('X') {
             if self.consume('?') {
-                spec.flags |= 1 << (FlagDebugUpperHex as u32);
+                spec.debug_hex = Some(DebugHex::Upper);
                 spec.ty = "?";
             } else {
                 spec.ty = "X";
@@ -708,7 +714,10 @@ impl<'a> Parser<'a> {
         let mut spec = FormatSpec {
             fill: None,
             align: AlignUnknown,
-            flags: 0,
+            sign: None,
+            alternate: false,
+            zero_pad: false,
+            debug_hex: None,
             precision: CountImplied,
             precision_span: None,
             width: CountImplied,
diff --git a/compiler/rustc_parse_format/src/tests.rs b/compiler/rustc_parse_format/src/tests.rs
diff --git a/src/tools/clippy/clippy_utils/src/macros.rs b/src/tools/clippy/clippy_utils/src/macros.rs

-Original file line number
+Diff line change
     pub fill: Option<char>,
     /// Optionally specified alignment.
     pub align: Alignment,
 -    /// Packed version of various flags provided, see [`rustc_parse_format::Flag`].
 -    pub flags: u32,
 +    /// Whether all flag options are set to default (no flags specified).
 +    pub no_flags: bool,
     /// Represents either the maximum width or the integer precision.
     pub precision: Count<'tcx>,
     /// The minimum width, will be padded according to `width`/`align`
         Some(Self {
             fill: spec.fill,
             align: spec.align,
 -            flags: spec.flags,
 +            no_flags: spec.sign.is_none() && !spec.alternate && !spec.zero_pad && spec.debug_hex.is_none(),
             precision: Count::new(
                 FormatParamUsage::Precision,
                 spec.precision,
         self.width.is_implied()
             && self.precision.is_implied()
             && self.align == Alignment::AlignUnknown
 -            && self.flags == 0
 +            && self.no_flags
+    }
+}