From 23411ce1661a5496821419e3c685e3abd4493e1f Mon Sep 17 00:00:00 2001 From: "Andrew V. Teylu" Date: Mon, 23 Mar 2026 08:47:54 +0000 Subject: [PATCH] Refactor asm pretty-print operand reordering and broaden regression coverage Extract the inline asm operand reorder/remap logic into a dedicated helper so `print_inline_asm` stays focused on assembling the printed argument list. This keeps the reordered-template logic local to the AST pretty printer while making the main printing path easier to read. Also extend the pretty regression test with a `const` operand case. That checks that non-register operands other than `in(reg)` are also moved ahead of explicit register operands and that the remapped template indices still match the new operand order. Signed-off-by: Andrew V. Teylu --- compiler/rustc_ast_pretty/src/pprust/state.rs | 120 +++++++++--------- tests/pretty/asm-operand-order.pp | 1 + tests/pretty/asm-operand-order.rs | 1 + 3 files changed, 63 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 92767f97c28e..4823189f7be3 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1612,21 +1612,10 @@ fn print_mac(&mut self, m: &ast::MacCall) { ); } - fn print_inline_asm(&mut self, asm: &ast::InlineAsm) { - enum AsmArg<'a> { - Template(String), - Operand(&'a InlineAsmOperand), - ClobberAbi(Symbol), - Options(InlineAsmOptions), - } - - // After macro expansion, named operands become positional. The grammar - // requires positional operands to precede explicit register operands, - // so we must reorder when any non-explicit operand follows an explicit - // one. When no reordering is needed, we use the original template - // string and operand order to avoid duplicating the Display logic in - // InlineAsmTemplatePiece. - let is_explicit_reg = |op: &InlineAsmOperand| -> bool { + fn inline_asm_template_and_operands<'asm>( + asm: &'asm ast::InlineAsm, + ) -> (String, Vec<&'asm InlineAsmOperand>) { + fn is_explicit_reg(op: &InlineAsmOperand) -> bool { match op { InlineAsmOperand::In { reg, .. } | InlineAsmOperand::Out { reg, .. } @@ -1638,9 +1627,14 @@ enum AsmArg<'a> { | InlineAsmOperand::Sym { .. } | InlineAsmOperand::Label { .. } => false, } - }; + } - // Check whether any non-explicit operand follows an explicit one. + // After macro expansion, named operands become positional. The grammar + // requires positional operands to precede explicit register operands, + // so we must reorder when any non-explicit operand follows an explicit + // one. When no reordering is needed, we use the original template + // string and operand order to avoid duplicating the Display logic in + // InlineAsmTemplatePiece. let needs_reorder = { let mut seen_explicit = false; asm.operands.iter().any(|(op, _)| { @@ -1653,53 +1647,61 @@ enum AsmArg<'a> { }) }; - let mut args = if needs_reorder { - let mut non_explicit: Vec = Vec::new(); - let mut explicit: Vec = Vec::new(); - for (i, (op, _)) in asm.operands.iter().enumerate() { - if is_explicit_reg(op) { - explicit.push(i); - } else { - non_explicit.push(i); - } - } + if !needs_reorder { + let template = InlineAsmTemplatePiece::to_string(&asm.template); + let operands = asm.operands.iter().map(|(op, _)| op).collect(); + return (template, operands); + } - // Build old-index → new-index mapping for template renumbering. - let mut old_to_new = vec![0usize; asm.operands.len()]; - for (new_idx, &old_idx) in non_explicit.iter().chain(explicit.iter()).enumerate() { - old_to_new[old_idx] = new_idx; + let mut non_explicit = Vec::new(); + let mut explicit = Vec::new(); + for (i, (op, _)) in asm.operands.iter().enumerate() { + if is_explicit_reg(op) { + explicit.push(i); + } else { + non_explicit.push(i); } + } + let order = non_explicit.into_iter().chain(explicit).collect::>(); - // Remap template placeholder indices and reuse the existing - // Display impl to build the template string. - let remapped: Vec<_> = asm - .template - .iter() - .map(|piece| match piece { - InlineAsmTemplatePiece::Placeholder { operand_idx, modifier, span } => { - InlineAsmTemplatePiece::Placeholder { - operand_idx: old_to_new[*operand_idx], - modifier: *modifier, - span: *span, - } + // Build old-index -> new-index mapping for template renumbering. + let mut old_to_new = vec![0usize; asm.operands.len()]; + for (new_idx, old_idx) in order.iter().copied().enumerate() { + old_to_new[old_idx] = new_idx; + } + + // Remap template placeholder indices and reuse the existing Display + // impl to build the template string. + let remapped = asm + .template + .iter() + .map(|piece| match piece { + InlineAsmTemplatePiece::Placeholder { operand_idx, modifier, span } => { + InlineAsmTemplatePiece::Placeholder { + operand_idx: old_to_new[*operand_idx], + modifier: *modifier, + span: *span, } - other => other.clone(), - }) - .collect(); + } + other => other.clone(), + }) + .collect::>(); + let template = InlineAsmTemplatePiece::to_string(&remapped); + let operands = order.iter().map(|&idx| &asm.operands[idx].0).collect(); + (template, operands) + } - let mut args = vec![AsmArg::Template(InlineAsmTemplatePiece::to_string(&remapped))]; - for &idx in &non_explicit { - args.push(AsmArg::Operand(&asm.operands[idx].0)); - } - for &idx in &explicit { - args.push(AsmArg::Operand(&asm.operands[idx].0)); - } - args - } else { - let mut args = vec![AsmArg::Template(InlineAsmTemplatePiece::to_string(&asm.template))]; - args.extend(asm.operands.iter().map(|(o, _)| AsmArg::Operand(o))); - args - }; + fn print_inline_asm(&mut self, asm: &ast::InlineAsm) { + enum AsmArg<'a> { + Template(String), + Operand(&'a InlineAsmOperand), + ClobberAbi(Symbol), + Options(InlineAsmOptions), + } + + let (template, operands) = Self::inline_asm_template_and_operands(asm); + let mut args = vec![AsmArg::Template(template)]; + args.extend(operands.into_iter().map(AsmArg::Operand)); for (abi, _) in &asm.clobber_abis { args.push(AsmArg::ClobberAbi(*abi)); } diff --git a/tests/pretty/asm-operand-order.pp b/tests/pretty/asm-operand-order.pp index 723191412d0f..949c18c6723a 100644 --- a/tests/pretty/asm-operand-order.pp +++ b/tests/pretty/asm-operand-order.pp @@ -14,5 +14,6 @@ pub fn main() { asm!("{0}", in(reg) 4, out("rax") _); asm!("{0}", in(reg) 4, out("rax") _, options(nostack)); asm!("{0} {1}", in(reg) 4, in(reg) 5, out("rax") _); + asm!("{0}", const 5, out("rax") _); } } diff --git a/tests/pretty/asm-operand-order.rs b/tests/pretty/asm-operand-order.rs index 05a994e38544..b0bdabd94678 100644 --- a/tests/pretty/asm-operand-order.rs +++ b/tests/pretty/asm-operand-order.rs @@ -9,5 +9,6 @@ pub fn main() { asm!("{val}", out("rax") _, val = in(reg) 4); asm!("{val}", out("rax") _, val = in(reg) 4, options(nostack)); asm!("{a} {b}", out("rax") _, a = in(reg) 4, b = in(reg) 5); + asm!("{val}", out("rax") _, val = const 5); } }