[debuginfo] Make use of spaces and separators in debuginfo names more uniform.

compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs

@@ -85,25 +85,14 @@ pub fn push_debuginfo_type_name<'tcx>(
 
             for component_type in component_types {
                 push_debuginfo_type_name(tcx, component_type.expect_ty(), true, output, visited);
-                output.push(',');
+                push_arg_separator(cpp_like_names, output);
-
-                // Natvis does not always like having spaces between parts of the type name
-                // and this causes issues when we need to write a typename in natvis, for example
-                // as part of a cast like the `HashMap` visualizer does.
-                if !cpp_like_names {
-                    output.push(' ');
-                }
             }
             if !component_types.is_empty() {
-                output.pop();
+                pop_arg_separator(output);
-
-                if !cpp_like_names {
-                    output.pop();
-                }
             }
 
             if cpp_like_names {
-                push_close_angle_bracket(tcx, output);
+                push_close_angle_bracket(cpp_like_names, output);
             } else {
                 output.push(')');
             }
@@ -125,7 +114,7 @@ pub fn push_debuginfo_type_name<'tcx>(
             push_debuginfo_type_name(tcx, inner_type, qualified, output, visited);
 
             if cpp_like_names {
-                push_close_angle_bracket(tcx, output);
+                push_close_angle_bracket(cpp_like_names, output);
             }
         }
         ty::Ref(_, inner_type, mutbl) => {
@@ -151,7 +140,7 @@ pub fn push_debuginfo_type_name<'tcx>(
             push_debuginfo_type_name(tcx, inner_type, qualified, output, visited);
 
             if cpp_like_names && !is_slice_or_str {
-                push_close_angle_bracket(tcx, output);
+                push_close_angle_bracket(cpp_like_names, output);
             }
         }
         ty::Array(inner_type, len) => {
@@ -183,7 +172,7 @@ pub fn push_debuginfo_type_name<'tcx>(
             push_debuginfo_type_name(tcx, inner_type, true, output, visited);
 
             if cpp_like_names {
-                push_close_angle_bracket(tcx, output);
+                push_close_angle_bracket(cpp_like_names, output);
             } else {
                 output.push(']');
             }
@@ -196,7 +185,7 @@ pub fn push_debuginfo_type_name<'tcx>(
                 false
             } else {
                 if trait_data.len() > 1 && auto_traits.len() != 0 {
-                    // We need enclosing parens
+                    // We need enclosing parens because there is more than one trait
                     output.push_str("(dyn ");
                     true
                 } else {
@@ -209,7 +198,8 @@ pub fn push_debuginfo_type_name<'tcx>(
                 let principal =
                     tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), principal);
                 push_item_name(tcx, principal.def_id, qualified, output);
-                push_generic_params_internal(tcx, principal.substs, output, visited);
+                let principal_has_generic_params =
+                    push_generic_params_internal(tcx, principal.substs, output, visited);
 
                 let projection_bounds: SmallVec<[_; 4]> = trait_data
                     .projection_bounds()
@@ -220,17 +210,21 @@ pub fn push_debuginfo_type_name<'tcx>(
                     .collect();
 
                 if projection_bounds.len() != 0 {
-                    pop_close_angle_bracket(output);
+                    if principal_has_generic_params {
+                        // push_generic_params_internal() above added a `>` but we actually
+                        // want to add more items to that list, so remove that again.
+                        pop_close_angle_bracket(output);
+                    }
 
                     for (item_def_id, ty) in projection_bounds {
-                        output.push_str(", ");
+                        push_arg_separator(cpp_like_names, output);
 
                         if cpp_like_names {
                             output.push_str("assoc$<");
                             push_item_name(tcx, item_def_id, false, output);
-                            output.push_str(", ");
+                            push_arg_separator(cpp_like_names, output);
                             push_debuginfo_type_name(tcx, ty, true, output, visited);
-                            push_close_angle_bracket(tcx, output);
+                            push_close_angle_bracket(cpp_like_names, output);
                         } else {
                             push_item_name(tcx, item_def_id, false, output);
                             output.push('=');
@@ -238,7 +232,7 @@ pub fn push_debuginfo_type_name<'tcx>(
                         }
                     }
 
-                    push_close_angle_bracket(tcx, output);
+                    push_close_angle_bracket(cpp_like_names, output);
                 }
 
                 if auto_traits.len() != 0 {
@@ -262,11 +256,11 @@ pub fn push_debuginfo_type_name<'tcx>(
                     push_auto_trait_separator(cpp_like_names, output);
                 }
 
-                pop_auto_trait_separator(cpp_like_names, output);
+                pop_auto_trait_separator(output);
             }
 
             if cpp_like_names {
-                push_close_angle_bracket(tcx, output);
+                push_close_angle_bracket(cpp_like_names, output);
             } else if has_enclosing_parens {
                 output.push(')');
             }
@@ -320,10 +314,9 @@ pub fn push_debuginfo_type_name<'tcx>(
             if !sig.inputs().is_empty() {
                 for &parameter_type in sig.inputs() {
                     push_debuginfo_type_name(tcx, parameter_type, true, output, visited);
-                    output.push_str(", ");
+                    push_arg_separator(cpp_like_names, output);
                 }
-                output.pop();
+                pop_arg_separator(output);
-                output.pop();
             }
 
             if sig.c_variadic {
@@ -429,21 +422,25 @@ fn msvc_enum_fallback(
                 output.push_str(&format!(", {}", variant));
             }
         }
-        push_close_angle_bracket(tcx, output);
+        push_close_angle_bracket(true, output);
     }
 
-    fn auto_trait_separator(cpp_like_names: bool) -> &'static str {
+    const NON_CPP_AUTO_TRAIT_SEPARATOR: &str = " + ";
-        if cpp_like_names { ", " } else { " + " }
-    }
 
     fn push_auto_trait_separator(cpp_like_names: bool, output: &mut String) {
-        output.push_str(auto_trait_separator(cpp_like_names));
+        if cpp_like_names {
+            push_arg_separator(cpp_like_names, output);
+        } else {
+            output.push_str(NON_CPP_AUTO_TRAIT_SEPARATOR);
+        }
     }
 
-    fn pop_auto_trait_separator(cpp_like_names: bool, output: &mut String) {
+    fn pop_auto_trait_separator(output: &mut String) {
-        let sep = auto_trait_separator(cpp_like_names);
+        if output.ends_with(NON_CPP_AUTO_TRAIT_SEPARATOR) {
-        assert!(output.ends_with(sep));
+            output.truncate(output.len() - NON_CPP_AUTO_TRAIT_SEPARATOR.len());
-        output.truncate(output.len() - sep.len());
+        } else {
+            pop_arg_separator(output);
+        }
     }
 }
 
@@ -504,13 +501,15 @@ fn push_generic_params_internal<'tcx>(
     substs: SubstsRef<'tcx>,
     output: &mut String,
     visited: &mut FxHashSet<Ty<'tcx>>,
-) {
+) -> bool {
     if substs.non_erasable_generics().next().is_none() {
-        return;
+        return false;
     }
 
     debug_assert_eq!(substs, tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), substs));
 
+    let cpp_like_names = cpp_like_names(tcx);
+
     output.push('<');
 
     for type_parameter in substs.non_erasable_generics() {
@@ -524,13 +523,12 @@ fn push_generic_params_internal<'tcx>(
             other => bug!("Unexpected non-erasable generic: {:?}", other),
         }
 
-        output.push_str(", ");
+        push_arg_separator(cpp_like_names, output);
     }
+    pop_arg_separator(output);
+    push_close_angle_bracket(cpp_like_names, output);
 
-    output.pop();
+    true
-    output.pop();
-
-    push_close_angle_bracket(tcx, output);
 }
 
 fn push_const_param<'tcx>(tcx: TyCtxt<'tcx>, ct: &'tcx ty::Const<'tcx>, output: &mut String) {
@@ -583,10 +581,10 @@ pub fn push_generic_params<'tcx>(tcx: TyCtxt<'tcx>, substs: SubstsRef<'tcx>, out
     push_generic_params_internal(tcx, substs, output, &mut visited);
 }
 
-fn push_close_angle_bracket<'tcx>(tcx: TyCtxt<'tcx>, output: &mut String) {
+fn push_close_angle_bracket(cpp_like_names: bool, output: &mut String) {
     // MSVC debugger always treats `>>` as a shift, even when parsing templates,
     // so add a space to avoid confusion.
-    if cpp_like_names(tcx) && output.ends_with('>') {
+    if cpp_like_names && output.ends_with('>') {
         output.push(' ')
     };
 
@@ -594,13 +592,34 @@ fn push_close_angle_bracket<'tcx>(tcx: TyCtxt<'tcx>, output: &mut String) {
 }
 
 fn pop_close_angle_bracket(output: &mut String) {
-    assert!(output.ends_with('>'));
+    assert!(output.ends_with('>'), "'output' does not end with '>': {}", output);
     output.pop();
     if output.ends_with(' ') {
         output.pop();
     }
 }
 
+fn push_arg_separator(cpp_like_names: bool, output: &mut String) {
+    // Natvis does not always like having spaces between parts of the type name
+    // and this causes issues when we need to write a typename in natvis, for example
+    // as part of a cast like the `HashMap` visualizer does.
+    if cpp_like_names {
+        output.push(',');
+    } else {
+        output.push_str(", ");
+    };
+}
+
+fn pop_arg_separator(output: &mut String) {
+    if output.ends_with(' ') {
+        output.pop();
+    }
+
+    assert!(output.ends_with(','));
+
+    output.pop();
+}
+
 fn cpp_like_names(tcx: TyCtxt<'_>) -> bool {
     tcx.sess.target.is_like_msvc
 }

src/test/debuginfo/function-names.rs

@@ -52,21 +52,21 @@
 // cdb-command:x a!function_names::*::impl_function*
 // cdb-check:[...] a!function_names::Mod1::TestStruct2::impl_function (void)
 // cdb-check:[...] a!function_names::TestStruct1::impl_function (void)
-// cdb-check:[...] a!function_names::GenericStruct<i32, i32>::impl_function<i32, i32> (void)
+// cdb-check:[...] a!function_names::GenericStruct<i32,i32>::impl_function<i32,i32> (void)
 
 // Trait implementations
 // cdb-command:x a!function_names::*::trait_function*
 // cdb-check:[...] a!function_names::impl$3::trait_function<i32> (void)
+// cdb-check:[...] a!function_names::impl$6::trait_function<i32,1> (void)
 // cdb-check:[...] a!function_names::impl$1::trait_function (void)
-// cdb-check:[...] a!function_names::impl$6::trait_function<i32, 1> (void)
 // cdb-check:[...] a!function_names::impl$5::trait_function3<function_names::TestStruct1> (void)
 // cdb-check:[...] a!function_names::Mod1::impl$1::trait_function (void)
 
 // Closure
 // cdb-command:x a!function_names::*::closure*
+// cdb-check:[...] a!function_names::impl$2::impl_function::closure$0<i32,i32> (void)
 // cdb-check:[...] a!function_names::main::closure$0 (void)
 // cdb-check:[...] a!function_names::generic_func::closure$0<i32> (void)
-// cdb-check:[...] a!function_names::impl$2::impl_function::closure$0<i32, i32> (void)
 
 // Generator
 // cdb-command:x a!function_names::*::generator*

src/test/debuginfo/type-names.rs

@@ -173,7 +173,7 @@
 // 0-sized structs appear to be optimized away in some cases, so only check the structs that do
 // actually appear.
 // cdb-command:dv /t *_struct
-// cdb-check:struct type_names::GenericStruct<enum$<type_names::mod1::Enum2>, f64> mut_generic_struct = [...]
+// cdb-check:struct type_names::GenericStruct<enum$<type_names::mod1::Enum2>,f64> mut_generic_struct = [...]
 
 // ENUMS
 // cdb-command:dv /t *_enum_*
@@ -190,15 +190,15 @@
 
 // BOX
 // cdb-command:dv /t box*
-// cdb-check:struct tuple$<alloc::boxed::Box<f32, alloc::alloc::Global>,i32> box1 = [...]
+// cdb-check:struct tuple$<alloc::boxed::Box<f32,alloc::alloc::Global>,i32> box1 = [...]
-// cdb-check:struct tuple$<alloc::boxed::Box<enum$<type_names::mod1::mod2::Enum3<f32> >, alloc::alloc::Global>,i32> box2 = [...]
+// cdb-check:struct tuple$<alloc::boxed::Box<enum$<type_names::mod1::mod2::Enum3<f32> >,alloc::alloc::Global>,i32> box2 = [...]
 
 // REFERENCES
 // cdb-command:dv /t *ref*
 // cdb-check:struct tuple$<ref$<type_names::Struct1>,i32> ref1 = [...]
-// cdb-check:struct tuple$<ref$<type_names::GenericStruct<char, type_names::Struct1> >,i32> ref2 = [...]
+// cdb-check:struct tuple$<ref$<type_names::GenericStruct<char,type_names::Struct1> >,i32> ref2 = [...]
 // cdb-check:struct tuple$<ref_mut$<type_names::Struct1>,i32> mut_ref1 = [...]
-// cdb-check:struct tuple$<ref_mut$<type_names::GenericStruct<enum$<type_names::mod1::Enum2>, f64> >,i32> mut_ref2 = [...]
+// cdb-check:struct tuple$<ref_mut$<type_names::GenericStruct<enum$<type_names::mod1::Enum2>,f64> >,i32> mut_ref2 = [...]
 
 // RAW POINTERS
 // cdb-command:dv /t *_ptr*
@@ -213,31 +213,31 @@
 // cdb-command:dv /t *vec*
 // cdb-check:struct tuple$<array$<type_names::Struct1,3>,i16> fixed_size_vec1 = [...]
 // cdb-check:struct tuple$<array$<usize,3>,i16> fixed_size_vec2 = [...]
-// cdb-check:struct alloc::vec::Vec<usize, alloc::alloc::Global> vec1 = [...]
+// cdb-check:struct alloc::vec::Vec<usize,alloc::alloc::Global> vec1 = [...]
-// cdb-check:struct alloc::vec::Vec<enum$<type_names::mod1::Enum2>, alloc::alloc::Global> vec2 = [...]
+// cdb-check:struct alloc::vec::Vec<enum$<type_names::mod1::Enum2>,alloc::alloc::Global> vec2 = [...]
 // cdb-command:dv /t slice*
 // cdb-check:struct slice$<usize> slice1 = [...]
 // cdb-check:struct slice$<enum$<type_names::mod1::Enum2> > slice2 = [...]
 
 // TRAITS
 // cdb-command:dv /t *_trait
-// cdb-check:struct ref_mut$<dyn$<type_names::Trait2<type_names::mod1::mod2::Struct3, type_names::GenericStruct<usize, isize> > > > generic_mut_ref_trait = [...]
+// cdb-check:struct ref_mut$<dyn$<type_names::Trait2<type_names::mod1::mod2::Struct3,type_names::GenericStruct<usize,isize> > > > generic_mut_ref_trait = [...]
-// cdb-check:struct ref$<dyn$<type_names::Trait2<type_names::Struct1, type_names::Struct1> > > generic_ref_trait = [...]
+// cdb-check:struct ref$<dyn$<type_names::Trait2<type_names::Struct1,type_names::Struct1> > > generic_ref_trait = [...]
-// cdb-check:struct alloc::boxed::Box<dyn$<type_names::Trait2<i32, type_names::mod1::Struct2> >, alloc::alloc::Global> generic_box_trait = [...]
+// cdb-check:struct alloc::boxed::Box<dyn$<type_names::Trait2<i32,type_names::mod1::Struct2> >,alloc::alloc::Global> generic_box_trait = [...]
-// cdb-check:struct alloc::boxed::Box<dyn$<type_names::Trait1>, alloc::alloc::Global> box_trait = [...]
+// cdb-check:struct alloc::boxed::Box<dyn$<type_names::Trait1>,alloc::alloc::Global> box_trait = [...]
 // cdb-check:struct ref$<dyn$<type_names::Trait1> > ref_trait = [...]
 // cdb-check:struct ref_mut$<dyn$<type_names::Trait1> > mut_ref_trait = [...]
-// cdb-check:struct alloc::boxed::Box<dyn$<core::marker::Send, core::marker::Sync>, alloc::alloc::Global> no_principal_trait = [...]
+// cdb-check:struct alloc::boxed::Box<dyn$<core::marker::Send,core::marker::Sync>,alloc::alloc::Global> no_principal_trait = [...]
-// cdb-check:struct ref$<dyn$<type_names::Trait3<u32, assoc$<AssocType, isize> >, core::marker::Send> > has_associated_type_trait = struct ref$<dyn$<type_names::Trait3<u32, assoc$<AssocType, isize> >, core::marker::Send> >
+// cdb-check:struct ref$<dyn$<type_names::Trait3<u32,assoc$<AssocType,isize> >,core::marker::Send> > has_associated_type_trait = struct ref$<dyn$<type_names::Trait3<u32,assoc$<AssocType,isize> >,core::marker::Send> >
 
 // BARE FUNCTIONS
 // cdb-command:dv /t *_fn*
-// cdb-check:struct tuple$<type_names::mod1::Struct2 (*)(type_names::GenericStruct<u16, u8>),usize> unsafe_fn_with_return_value = [...]
+// cdb-check:struct tuple$<type_names::mod1::Struct2 (*)(type_names::GenericStruct<u16,u8>),usize> unsafe_fn_with_return_value = [...]
 // cdb-check:struct tuple$<type_names::Struct1 (*)(),usize> extern_c_fn_with_return_value = [...]
 // cdb-check:struct tuple$<usize (*)(f64),usize> rust_fn_with_return_value = [...]
-// cdb-check:struct tuple$<void (*)(enum$<core::result::Result<char, f64> >),usize> unsafe_fn = [...]
+// cdb-check:struct tuple$<void (*)(enum$<core::result::Result<char,f64> >),usize> unsafe_fn = [...]
 // cdb-check:struct tuple$<void (*)(isize),usize> extern_c_fn = [...]
-// cdb-check:struct tuple$<void (*)(enum$<core::option::Option<isize> >, enum$<core::option::Option<ref$<type_names::mod1::Struct2> >, 1, [...], Some>),usize> rust_fn = [...]
+// cdb-check:struct tuple$<void (*)(enum$<core::option::Option<isize> >,enum$<core::option::Option<ref$<type_names::mod1::Struct2> >, 1, [...], Some>),usize> rust_fn = [...]
 // cdb-command:dv /t *_function*
 // cdb-check:struct tuple$<isize (*)(ptr_const$<u8>, ...),usize> variadic_function = [...]
 // cdb-check:struct tuple$<type_names::mod1::mod2::Struct3 (*)(type_names::mod1::mod2::Struct3),usize> generic_function_struct3 = [...]