Auto merge of rust-lang#145284 - nnethercote:type_name-print-regions, r=lcnr

Print regions in `type_name`.

Currently they are skipped, which is a bit weird, and it sometimes causes malformed output like `Foo<>` and `dyn Bar<, A = u32>`.

Most regions are erased by the time `type_name` does its work. So all regions are now printed as `'_` in non-optional places. Not perfect, but better than the status quo.

`c_name` is updated to trim lifetimes from MIR pass names, so that the `PASS_NAMES` sanity check still works. It is also renamed as `simplify_pass_type_name` and made non-const, because it doesn't need to be const and the non-const implementation is much shorter.

The commit also renames `should_print_region` as `should_print_optional_region`, which makes it clearer that it only applies to some regions.

Fixes rust-lang#145168.

r? `@lcnr`

Author: bors

compiler/rustc_const_eval/src/util/type_name.rs

@@ -5,7 +5,7 @@ use rustc_hir::def_id::CrateNum;
 use rustc_hir::definitions::DisambiguatedDefPathData;
 use rustc_middle::bug;
 use rustc_middle::ty::print::{PrettyPrinter, PrintError, Printer};
-use rustc_middle::ty::{self, GenericArg, GenericArgKind, Ty, TyCtxt};
+use rustc_middle::ty::{self, GenericArg, Ty, TyCtxt};
 
 struct TypeNamePrinter<'tcx> {
     tcx: TyCtxt<'tcx>,
@@ -18,9 +18,10 @@ impl<'tcx> Printer<'tcx> for TypeNamePrinter<'tcx> {
     }
 
     fn print_region(&mut self, _region: ty::Region<'_>) -> Result<(), PrintError> {
-        // This is reachable (via `pretty_print_dyn_existential`) even though
-        // `<Self As PrettyPrinter>::should_print_region` returns false. See #144994.
-        Ok(())
+        // FIXME: most regions have been erased by the time this code runs.
+        // Just printing `'_` is a bit hacky but gives mostly good results, and
+        // doing better is difficult. See `should_print_optional_region`.
+        write!(self, "'_")
     }
 
     fn print_type(&mut self, ty: Ty<'tcx>) -> Result<(), PrintError> {
@@ -125,19 +126,24 @@ impl<'tcx> Printer<'tcx> for TypeNamePrinter<'tcx> {
         args: &[GenericArg<'tcx>],
     ) -> Result<(), PrintError> {
         print_prefix(self)?;
-        let args =
-            args.iter().cloned().filter(|arg| !matches!(arg.kind(), GenericArgKind::Lifetime(_)));
-        if args.clone().next().is_some() {
-            self.generic_delimiters(|cx| cx.comma_sep(args))
+        if !args.is_empty() {
+            self.generic_delimiters(|cx| cx.comma_sep(args.iter().copied()))
         } else {
             Ok(())
         }
     }
 }
 
 impl<'tcx> PrettyPrinter<'tcx> for TypeNamePrinter<'tcx> {
-    fn should_print_region(&self, _region: ty::Region<'_>) -> bool {
-        false
+    fn should_print_optional_region(&self, _region: ty::Region<'_>) -> bool {
+        // Bound regions are always printed (as `'_`), which gives some idea that they are special,
+        // even though the `for` is omitted by the pretty printer.
+        // E.g. `for<'a, 'b> fn(&'a u32, &'b u32)` is printed as "fn(&'_ u32, &'_ u32)".
+        match _region.kind() {
+            ty::ReErased => false,
+            ty::ReBound(..) => true,
+            _ => unreachable!(),
+        }
     }
 
     fn generic_delimiters(

compiler/rustc_lint/src/context.rs

@@ -756,22 +756,22 @@ impl<'tcx> LateContext<'tcx> {
             }
 
             fn print_region(&mut self, _region: ty::Region<'_>) -> Result<(), PrintError> {
-                unreachable!(); // because `path_generic_args` ignores the `GenericArgs`
+                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
             }
 
             fn print_type(&mut self, _ty: Ty<'tcx>) -> Result<(), PrintError> {
-                unreachable!(); // because `path_generic_args` ignores the `GenericArgs`
+                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
             }
 
             fn print_dyn_existential(
                 &mut self,
                 _predicates: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
             ) -> Result<(), PrintError> {
-                unreachable!(); // because `path_generic_args` ignores the `GenericArgs`
+                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
             }
 
             fn print_const(&mut self, _ct: ty::Const<'tcx>) -> Result<(), PrintError> {
-                unreachable!(); // because `path_generic_args` ignores the `GenericArgs`
+                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
             }
 
             fn print_crate_name(&mut self, cnum: CrateNum) -> Result<(), PrintError> {

compiler/rustc_middle/src/ty/print/pretty.rs

@@ -337,10 +337,10 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
         false
     }
 
-    /// Returns `true` if the region should be printed in
-    /// optional positions, e.g., `&'a T` or `dyn Tr + 'b`.
-    /// This is typically the case for all non-`'_` regions.
-    fn should_print_region(&self, region: ty::Region<'tcx>) -> bool;
+    /// Returns `true` if the region should be printed in optional positions,
+    /// e.g., `&'a T` or `dyn Tr + 'b`. (Regions like the one in `Cow<'static, T>`
+    /// will always be printed.)
+    fn should_print_optional_region(&self, region: ty::Region<'tcx>) -> bool;
 
     fn reset_type_limit(&mut self) {}
 
@@ -717,7 +717,7 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
             }
             ty::Ref(r, ty, mutbl) => {
                 write!(self, "&")?;
-                if self.should_print_region(r) {
+                if self.should_print_optional_region(r) {
                     r.print(self)?;
                     write!(self, " ")?;
                 }
@@ -785,7 +785,7 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
             },
             ty::Adt(def, args) => self.print_def_path(def.did(), args)?,
             ty::Dynamic(data, r, repr) => {
-                let print_r = self.should_print_region(r);
+                let print_r = self.should_print_optional_region(r);
                 if print_r {
                     write!(self, "(")?;
                 }
@@ -2494,7 +2494,7 @@ impl<'tcx> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx> {
         !self.type_length_limit.value_within_limit(self.printed_type_count)
     }
 
-    fn should_print_region(&self, region: ty::Region<'tcx>) -> bool {
+    fn should_print_optional_region(&self, region: ty::Region<'tcx>) -> bool {
         let highlight = self.region_highlight_mode;
         if highlight.region_highlighted(region).is_some() {
             return true;

compiler/rustc_mir_transform/src/pass_manager.rs

@@ -41,19 +41,40 @@ fn to_profiler_name(type_name: &'static str) -> &'static str {
     })
 }
 
-// const wrapper for `if let Some((_, tail)) = name.rsplit_once(':') { tail } else { name }`
-const fn c_name(name: &'static str) -> &'static str {
+// A function that simplifies a pass's type_name. E.g. `Baz`, `Baz<'_>`,
+// `foo::bar::Baz`, and `foo::bar::Baz<'a, 'b>` all become `Baz`.
+//
+// It's `const` for perf reasons: it's called a lot, and doing the string
+// operations at runtime causes a non-trivial slowdown. If
+// `split_once`/`rsplit_once` become `const` its body could be simplified to
+// this:
+// ```ignore (fragment)
+// let name = if let Some((_, tail)) = name.rsplit_once(':') { tail } else { name };
+// let name = if let Some((head, _)) = name.split_once('<') { head } else { name };
+// name
+// ```
+const fn simplify_pass_type_name(name: &'static str) -> &'static str {
     // FIXME(const-hack) Simplify the implementation once more `str` methods get const-stable.
-    // and inline into call site
+
+    // Work backwards from the end. If a ':' is hit, strip it and everything before it.
     let bytes = name.as_bytes();
     let mut i = bytes.len();
     while i > 0 && bytes[i - 1] != b':' {
-        i = i - 1;
+        i -= 1;
     }
     let (_, bytes) = bytes.split_at(i);
+
+    // Work forwards from the start of what's left. If a '<' is hit, strip it and everything after
+    // it.
+    let mut i = 0;
+    while i < bytes.len() && bytes[i] != b'<' {
+        i += 1;
+    }
+    let (bytes, _) = bytes.split_at(i);
+
     match std::str::from_utf8(bytes) {
         Ok(name) => name,
-        Err(_) => name,
+        Err(_) => panic!(),
     }
 }
 
@@ -62,12 +83,7 @@ const fn c_name(name: &'static str) -> &'static str {
 /// loop that goes over each available MIR and applies `run_pass`.
 pub(super) trait MirPass<'tcx> {
     fn name(&self) -> &'static str {
-        // FIXME(const-hack) Simplify the implementation once more `str` methods get const-stable.
-        // See copypaste in `MirLint`
-        const {
-            let name = std::any::type_name::<Self>();
-            c_name(name)
-        }
+        const { simplify_pass_type_name(std::any::type_name::<Self>()) }
     }
 
     fn profiler_name(&self) -> &'static str {
@@ -101,12 +117,7 @@ pub(super) trait MirPass<'tcx> {
 /// disabled (via the `Lint` adapter).
 pub(super) trait MirLint<'tcx> {
     fn name(&self) -> &'static str {
-        // FIXME(const-hack) Simplify the implementation once more `str` methods get const-stable.
-        // See copypaste in `MirPass`
-        const {
-            let name = std::any::type_name::<Self>();
-            c_name(name)
-        }
+        const { simplify_pass_type_name(std::any::type_name::<Self>()) }
     }
 
     fn is_enabled(&self, _sess: &Session) -> bool {

compiler/rustc_symbol_mangling/src/legacy.rs

@@ -235,7 +235,8 @@ impl<'tcx> Printer<'tcx> for LegacySymbolMangler<'tcx> {
 
     fn print_region(&mut self, _region: ty::Region<'_>) -> Result<(), PrintError> {
         // This might be reachable (via `pretty_print_dyn_existential`) even though
-        // `<Self As PrettyPrinter>::should_print_region` returns false. See #144994.
+        // `<Self As PrettyPrinter>::should_print_optional_region` returns false and
+        // `print_path_with_generic_args` filters out lifetimes. See #144994.
         Ok(())
     }
 
@@ -389,7 +390,6 @@ impl<'tcx> Printer<'tcx> for LegacySymbolMangler<'tcx> {
 
         let args =
             args.iter().cloned().filter(|arg| !matches!(arg.kind(), GenericArgKind::Lifetime(_)));
-
         if args.clone().next().is_some() {
             self.generic_delimiters(|cx| cx.comma_sep(args))
         } else {
@@ -459,7 +459,7 @@ impl<'tcx> Printer<'tcx> for LegacySymbolMangler<'tcx> {
 }
 
 impl<'tcx> PrettyPrinter<'tcx> for LegacySymbolMangler<'tcx> {
-    fn should_print_region(&self, _region: ty::Region<'_>) -> bool {
+    fn should_print_optional_region(&self, _region: ty::Region<'_>) -> bool {
         false
     }
 

compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs

@@ -235,22 +235,22 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
             }
 
             fn print_region(&mut self, _region: ty::Region<'_>) -> Result<(), PrintError> {
-                unreachable!(); // because `path_generic_args` ignores the `GenericArgs`
+                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
             }
 
             fn print_type(&mut self, _ty: Ty<'tcx>) -> Result<(), PrintError> {
-                unreachable!(); // because `path_generic_args` ignores the `GenericArgs`
+                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
             }
 
             fn print_dyn_existential(
                 &mut self,
                 _predicates: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
             ) -> Result<(), PrintError> {
-                unreachable!(); // because `path_generic_args` ignores the `GenericArgs`
+                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
             }
 
             fn print_const(&mut self, _ct: ty::Const<'tcx>) -> Result<(), PrintError> {
-                unreachable!(); // because `path_generic_args` ignores the `GenericArgs`
+                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
             }
 
             fn print_crate_name(&mut self, cnum: CrateNum) -> Result<(), PrintError> {

library/core/src/any.rs

@@ -835,9 +835,9 @@ impl fmt::Debug for TypeId {
 ///
 /// The returned string must not be considered to be a unique identifier of a
 /// type as multiple types may map to the same type name. Similarly, there is no
-/// guarantee that all parts of a type will appear in the returned string: for
-/// example, lifetime specifiers are currently not included. In addition, the
-/// output may change between versions of the compiler.
+/// guarantee that all parts of a type will appear in the returned string. In
+/// addition, the output may change between versions of the compiler. For
+/// example, lifetime specifiers were omitted in some earlier versions.
 ///
 /// The current implementation uses the same infrastructure as compiler
 /// diagnostics and debuginfo, but this is not guaranteed.

tests/ui/type/type-name-basic.rs

@@ -62,28 +62,32 @@ pub fn main() {
 
     t!(Vec<Vec<u32>>, "alloc::vec::Vec<alloc::vec::Vec<u32>>");
     t!(Foo<usize>, "type_name_basic::Foo<usize>");
-    t!(Bar<'static>, "type_name_basic::Bar");
-    t!(Baz<'static, u32>, "type_name_basic::Baz<u32>");
+    t!(Bar<'static>, "type_name_basic::Bar<'_>");
+    t!(Baz<'static, u32>, "type_name_basic::Baz<'_, u32>");
 
-    // FIXME: lifetime omission means these all print badly.
-    t!(dyn TrL<'static>, "dyn type_name_basic::TrL<>");
-    t!(dyn TrLA<'static, A = u32>, "dyn type_name_basic::TrLA<, A = u32>");
+    t!(dyn TrL<'static>, "dyn type_name_basic::TrL<'_>");
+    t!(dyn TrLA<'static, A = u32>, "dyn type_name_basic::TrLA<'_, A = u32>");
     t!(
         dyn TrLT<'static, Cow<'static, ()>>,
-        "dyn type_name_basic::TrLT<, alloc::borrow::Cow<()>>"
+        "dyn type_name_basic::TrLT<'_, alloc::borrow::Cow<'_, ()>>"
     );
     t!(
         dyn TrLTA<'static, u32, A = Cow<'static, ()>>,
-        "dyn type_name_basic::TrLTA<, u32, A = alloc::borrow::Cow<()>>"
+        "dyn type_name_basic::TrLTA<'_, u32, A = alloc::borrow::Cow<'_, ()>>"
     );
 
     t!(fn(i32) -> i32, "fn(i32) -> i32");
-    t!(dyn for<'a> Fn(&'a u32), "dyn core::ops::function::Fn(&u32)");
+    t!(fn(&'static u32), "fn(&u32)");
+
+    // FIXME: these are sub-optimal, ideally the `for<...>` would be printed.
+    t!(for<'a> fn(&'a u32), "fn(&'_ u32)");
+    t!(for<'a, 'b> fn(&'a u32, &'b u32), "fn(&'_ u32, &'_ u32)");
+    t!(for<'a> fn(for<'b> fn(&'a u32, &'b u32)), "fn(fn(&'_ u32, &'_ u32))");
 
     struct S<'a, T>(&'a T);
     impl<'a, T: Clone> S<'a, T> {
         fn test() {
-            t!(Cow<'a, T>, "alloc::borrow::Cow<u32>");
+            t!(Cow<'a, T>, "alloc::borrow::Cow<'_, u32>");
         }
     }
     S::<u32>::test();