From 9975d4760d1888a7cbd4bd9acfd26101792a3bcf Mon Sep 17 00:00:00 2001 From: "Andrew V. Teylu" Date: Thu, 19 Mar 2026 10:27:36 +0000 Subject: [PATCH 1/2] Reorder inline asm operands in pretty printer to satisfy grammar constraints After macro expansion, named `asm!` operands are converted to positional operands and the template string uses numeric indices. However, the pretty printer previously emitted operands in their original AST order, which could place positional (formerly named) register-class operands after explicit register operands. This violates the `asm!` grammar rule that positional arguments cannot follow explicit register arguments, causing the expanded output from `rustc -Zunpretty=expanded` to fail to reparse. When reordering is needed, the fix partitions operands into non-explicit and explicit register groups, emits non-explicit operands first, then explicit register operands, and remaps template placeholder indices (`{N}`) to match the new positions. When operands are already correctly ordered, the original code path is used unchanged. Signed-off-by: Andrew V. Teylu --- compiler/rustc_ast_pretty/src/pprust/state.rs | 82 ++++++++++++++++++- tests/pretty/asm-operand-order.pp | 18 ++++ tests/pretty/asm-operand-order.rs | 13 +++ 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 tests/pretty/asm-operand-order.pp create mode 100644 tests/pretty/asm-operand-order.rs diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 4ba5dc541342..92767f97c28e 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1620,8 +1620,86 @@ enum AsmArg<'a> { Options(InlineAsmOptions), } - let mut args = vec![AsmArg::Template(InlineAsmTemplatePiece::to_string(&asm.template))]; - args.extend(asm.operands.iter().map(|(o, _)| AsmArg::Operand(o))); + // 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 { + match op { + InlineAsmOperand::In { reg, .. } + | InlineAsmOperand::Out { reg, .. } + | InlineAsmOperand::InOut { reg, .. } + | InlineAsmOperand::SplitInOut { reg, .. } => { + matches!(reg, InlineAsmRegOrRegClass::Reg(_)) + } + InlineAsmOperand::Const { .. } + | InlineAsmOperand::Sym { .. } + | InlineAsmOperand::Label { .. } => false, + } + }; + + // Check whether any non-explicit operand follows an explicit one. + let needs_reorder = { + let mut seen_explicit = false; + asm.operands.iter().any(|(op, _)| { + if is_explicit_reg(op) { + seen_explicit = true; + false + } else { + seen_explicit + } + }) + }; + + 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); + } + } + + // 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; + } + + // 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, + } + } + other => other.clone(), + }) + .collect(); + + 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 + }; 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 new file mode 100644 index 000000000000..723191412d0f --- /dev/null +++ b/tests/pretty/asm-operand-order.pp @@ -0,0 +1,18 @@ +#![feature(prelude_import)] +#![no_std] +extern crate std; +#[prelude_import] +use ::std::prelude::rust_2015::*; +//@ pretty-mode:expanded +//@ pp-exact:asm-operand-order.pp +//@ only-x86_64 + +use std::arch::asm; + +pub fn main() { + unsafe { + 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") _); + } +} diff --git a/tests/pretty/asm-operand-order.rs b/tests/pretty/asm-operand-order.rs new file mode 100644 index 000000000000..05a994e38544 --- /dev/null +++ b/tests/pretty/asm-operand-order.rs @@ -0,0 +1,13 @@ +//@ pretty-mode:expanded +//@ pp-exact:asm-operand-order.pp +//@ only-x86_64 + +use std::arch::asm; + +pub fn main() { + unsafe { + 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); + } +} From 23411ce1661a5496821419e3c685e3abd4493e1f Mon Sep 17 00:00:00 2001 From: "Andrew V. Teylu" Date: Mon, 23 Mar 2026 08:47:54 +0000 Subject: [PATCH 2/2] 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); } }