Auto merge of rust-lang#73996 - da-x:short-unique-paths, r=petrochenkov

diagnostics: shorten paths of unique symbols

This is a step towards implementing a fix for rust-lang#50310, and continuation of the discussion in [Pre-RFC: Nicer Types In Diagnostics - compiler - Rust Internals](https://internals.rust-lang.org/t/pre-rfc-nicer-types-in-diagnostics/11139). Impressed upon me from previous discussion in rust-lang#21934 that an RFC for this is not needed, and I should just come up with code.

The recent improvements to `use` suggestions that I've contributed have given rise to this implementation. Contrary to previous suggestions, it's rather simple logic, and I believe it only reduces the amount of cognitive load that a developer would need when reading type errors.

-----

If a symbol name can only be imported from one place, and as long as it was not glob-imported anywhere in the current crate, we can trim its printed path to the last component.

This has wide implications on error messages with types, for example, shortening `std::vec::Vec` to just `Vec`, as long as there is no other `Vec` importable from anywhere.

compiler/rustc_codegen_llvm/src/type_of.rs

@@ -4,6 +4,7 @@ use crate::type_::Type;
 use rustc_codegen_ssa::traits::*;
 use rustc_middle::bug;
 use rustc_middle::ty::layout::{FnAbiExt, TyAndLayout};
+use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self, Ty, TypeFoldable};
 use rustc_target::abi::{Abi, AddressSpace, Align, FieldsShape};
 use rustc_target::abi::{Int, Pointer, F32, F64};
@@ -57,7 +58,7 @@ fn uncached_llvm_type<'a, 'tcx>(
         ty::Adt(..) | ty::Closure(..) | ty::Foreign(..) | ty::Generator(..) | ty::Str
             if !cx.sess().fewer_names() =>
         {
-            let mut name = layout.ty.to_string();
+            let mut name = with_no_trimmed_paths(|| layout.ty.to_string());
             if let (&ty::Adt(def, _), &Variants::Single { index }) =
                 (&layout.ty.kind, &layout.variants)
             {

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -16,6 +16,7 @@ use rustc_middle::mir;
 use rustc_middle::mir::interpret::{AllocId, ConstValue, Pointer, Scalar};
 use rustc_middle::mir::AssertKind;
 use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt};
+use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
 use rustc_span::source_map::Span;
 use rustc_span::{sym, Symbol};
@@ -479,14 +480,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                 UninitValid => !layout.might_permit_raw_init(bx, /*zero:*/ false).unwrap(),
             };
             if do_panic {
-                let msg_str = if layout.abi.is_uninhabited() {
-                    // Use this error even for the other intrinsics as it is more precise.
-                    format!("attempted to instantiate uninhabited type `{}`", ty)
-                } else if intrinsic == ZeroValid {
-                    format!("attempted to zero-initialize type `{}`, which is invalid", ty)
-                } else {
-                    format!("attempted to leave type `{}` uninitialized, which is invalid", ty)
-                };
+                let msg_str = with_no_trimmed_paths(|| {
+                    if layout.abi.is_uninhabited() {
+                        // Use this error even for the other intrinsics as it is more precise.
+                        format!("attempted to instantiate uninhabited type `{}`", ty)
+                    } else if intrinsic == ZeroValid {
+                        format!("attempted to zero-initialize type `{}`, which is invalid", ty)
+                    } else {
+                        format!("attempted to leave type `{}` uninitialized, which is invalid", ty)
+                    }
+                });
                 let msg = bx.const_str(Symbol::intern(&msg_str));
                 let location = self.get_caller_location(bx, span).immediate();
 

compiler/rustc_driver/src/lib.rs

@@ -32,7 +32,7 @@ use rustc_save_analysis as save;
 use rustc_save_analysis::DumpHandler;
 use rustc_serialize::json::{self, ToJson};
 use rustc_session::config::nightly_options;
-use rustc_session::config::{ErrorOutputType, Input, OutputType, PrintRequest};
+use rustc_session::config::{ErrorOutputType, Input, OutputType, PrintRequest, TrimmedDefPaths};
 use rustc_session::getopts;
 use rustc_session::lint::{Lint, LintId};
 use rustc_session::{config, DiagnosticOutput, Session};
@@ -126,6 +126,7 @@ impl Callbacks for TimePassesCallbacks {
         // time because it will mess up the --prints output. See #64339.
         self.time_passes = config.opts.prints.is_empty()
             && (config.opts.debugging_opts.time_passes || config.opts.debugging_opts.time);
+        config.opts.trimmed_def_paths = TrimmedDefPaths::GoodPath;
     }
 }
 

compiler/rustc_errors/src/lib.rs

@@ -4,6 +4,7 @@
 
 #![doc(html_root_url = "https://doc.rust-lang.org/nightly/")]
 #![feature(crate_visibility_modifier)]
+#![feature(backtrace)]
 #![feature(nll)]
 
 #[macro_use]
@@ -296,9 +297,11 @@ struct HandlerInner {
     /// This is not necessarily the count that's reported to the user once
     /// compilation ends.
     err_count: usize,
+    warn_count: usize,
     deduplicated_err_count: usize,
     emitter: Box<dyn Emitter + sync::Send>,
     delayed_span_bugs: Vec<Diagnostic>,
+    delayed_good_path_bugs: Vec<Diagnostic>,
 
     /// This set contains the `DiagnosticId` of all emitted diagnostics to avoid
     /// emitting the same diagnostic with extended help (`--teach`) twice, which
@@ -361,13 +364,15 @@ impl Drop for HandlerInner {
 
         if !self.has_errors() {
             let bugs = std::mem::replace(&mut self.delayed_span_bugs, Vec::new());
-            let has_bugs = !bugs.is_empty();
-            for bug in bugs {
-                self.emit_diagnostic(&bug);
-            }
-            if has_bugs {
-                panic!("no errors encountered even though `delay_span_bug` issued");
-            }
+            self.flush_delayed(bugs, "no errors encountered even though `delay_span_bug` issued");
+        }
+
+        if !self.has_any_message() {
+            let bugs = std::mem::replace(&mut self.delayed_good_path_bugs, Vec::new());
+            self.flush_delayed(
+                bugs,
+                "no warnings or errors encountered even though `delayed_good_path_bugs` issued",
+            );
         }
     }
 }
@@ -422,10 +427,12 @@ impl Handler {
             inner: Lock::new(HandlerInner {
                 flags,
                 err_count: 0,
+                warn_count: 0,
                 deduplicated_err_count: 0,
                 deduplicated_warn_count: 0,
                 emitter,
                 delayed_span_bugs: Vec::new(),
+                delayed_good_path_bugs: Vec::new(),
                 taught_diagnostics: Default::default(),
                 emitted_diagnostic_codes: Default::default(),
                 emitted_diagnostics: Default::default(),
@@ -448,11 +455,13 @@ impl Handler {
     pub fn reset_err_count(&self) {
         let mut inner = self.inner.borrow_mut();
         inner.err_count = 0;
+        inner.warn_count = 0;
         inner.deduplicated_err_count = 0;
         inner.deduplicated_warn_count = 0;
 
         // actually free the underlying memory (which `clear` would not do)
         inner.delayed_span_bugs = Default::default();
+        inner.delayed_good_path_bugs = Default::default();
         inner.taught_diagnostics = Default::default();
         inner.emitted_diagnostic_codes = Default::default();
         inner.emitted_diagnostics = Default::default();
@@ -629,6 +638,10 @@ impl Handler {
         self.inner.borrow_mut().delay_span_bug(span, msg)
     }
 
+    pub fn delay_good_path_bug(&self, msg: &str) {
+        self.inner.borrow_mut().delay_good_path_bug(msg)
+    }
+
     pub fn span_bug_no_panic(&self, span: impl Into<MultiSpan>, msg: &str) {
         self.emit_diag_at_span(Diagnostic::new(Bug, msg), span);
     }
@@ -768,6 +781,8 @@ impl HandlerInner {
         }
         if diagnostic.is_error() {
             self.bump_err_count();
+        } else {
+            self.bump_warn_count();
         }
     }
 
@@ -859,6 +874,9 @@ impl HandlerInner {
     fn has_errors_or_delayed_span_bugs(&self) -> bool {
         self.has_errors() || !self.delayed_span_bugs.is_empty()
     }
+    fn has_any_message(&self) -> bool {
+        self.err_count() > 0 || self.warn_count > 0
+    }
 
     fn abort_if_errors(&mut self) {
         self.emit_stashed_diagnostics();
@@ -892,6 +910,15 @@ impl HandlerInner {
         self.delay_as_bug(diagnostic)
     }
 
+    fn delay_good_path_bug(&mut self, msg: &str) {
+        let mut diagnostic = Diagnostic::new(Level::Bug, msg);
+        if self.flags.report_delayed_bugs {
+            self.emit_diagnostic(&diagnostic);
+        }
+        diagnostic.note(&format!("delayed at {}", std::backtrace::Backtrace::force_capture()));
+        self.delayed_good_path_bugs.push(diagnostic);
+    }
+
     fn failure(&mut self, msg: &str) {
         self.emit_diagnostic(&Diagnostic::new(FailureNote, msg));
     }
@@ -925,11 +952,25 @@ impl HandlerInner {
         self.delayed_span_bugs.push(diagnostic);
     }
 
+    fn flush_delayed(&mut self, bugs: Vec<Diagnostic>, explanation: &str) {
+        let has_bugs = !bugs.is_empty();
+        for bug in bugs {
+            self.emit_diagnostic(&bug);
+        }
+        if has_bugs {
+            panic!("{}", explanation);
+        }
+    }
+
     fn bump_err_count(&mut self) {
         self.err_count += 1;
         self.panic_if_treat_err_as_bug();
     }
 
+    fn bump_warn_count(&mut self) {
+        self.warn_count += 1;
+    }
+
     fn panic_if_treat_err_as_bug(&self) {
         if self.treat_err_as_bug() {
             let s = match (self.err_count(), self.flags.treat_err_as_bug.unwrap_or(0)) {

compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs

@@ -611,11 +611,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                     let sig = self.tcx.fn_sig(did);
                     let bound_output = sig.output();
                     let output = bound_output.skip_binder();
-                    err.span_label(e.span, &format!("this method call resolves to `{:?}`", output));
+                    err.span_label(e.span, &format!("this method call resolves to `{}`", output));
                     let kind = &output.kind;
                     if let ty::Projection(proj) = kind {
                         if let Some(span) = self.tcx.hir().span_if_local(proj.item_def_id) {
-                            err.span_label(span, &format!("`{:?}` defined here", output));
+                            err.span_label(span, &format!("`{}` defined here", output));
                         }
                     }
                 }

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/trait_impl_difference.rs

@@ -58,8 +58,8 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
             .tcx()
             .sess
             .struct_span_err(sp, "`impl` item signature doesn't match `trait` item signature");
-        err.span_label(sp, &format!("found `{:?}`", found));
-        err.span_label(trait_sp, &format!("expected `{:?}`", expected));
+        err.span_label(sp, &format!("found `{}`", found));
+        err.span_label(trait_sp, &format!("expected `{}`", expected));
 
         // Get the span of all the used type parameters in the method.
         let assoc_item = self.tcx().associated_item(trait_def_id);
@@ -92,7 +92,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
             err.note_expected_found(&"", expected, &"", found);
         } else {
             // This fallback shouldn't be necessary, but let's keep it in just in case.
-            err.note(&format!("expected `{:?}`\n   found `{:?}`", expected, found));
+            err.note(&format!("expected `{}`\n   found `{}`", expected, found));
         }
         err.span_help(
             type_param_span,

compiler/rustc_interface/src/tests.rs

@@ -518,6 +518,7 @@ fn test_debugging_options_tracking_hash() {
     untracked!(time_llvm_passes, true);
     untracked!(time_passes, true);
     untracked!(trace_macros, true);
+    untracked!(trim_diagnostic_paths, false);
     untracked!(ui_testing, true);
     untracked!(unpretty, Some("expanded".to_string()));
     untracked!(unstable_options, true);

compiler/rustc_lint/src/builtin.rs

@@ -40,6 +40,7 @@ use rustc_hir::{ForeignItemKind, GenericParamKind, PatKind};
 use rustc_hir::{HirId, HirIdSet, Node};
 use rustc_index::vec::Idx;
 use rustc_middle::lint::LintDiagnosticBuilder;
+use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::subst::{GenericArgKind, Subst};
 use rustc_middle::ty::{self, layout::LayoutError, Ty, TyCtxt};
 use rustc_session::lint::FutureIncompatibleInfo;
@@ -2040,7 +2041,9 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
             // using zeroed or uninitialized memory.
             // We are extremely conservative with what we warn about.
             let conjured_ty = cx.typeck_results().expr_ty(expr);
-            if let Some((msg, span)) = ty_find_init_error(cx.tcx, conjured_ty, init) {
+            if let Some((msg, span)) =
+                with_no_trimmed_paths(|| ty_find_init_error(cx.tcx, conjured_ty, init))
+            {
                 cx.struct_span_lint(INVALID_VALUE, expr.span, |lint| {
                     let mut err = lint.build(&format!(
                         "the type `{}` does not permit {}",

compiler/rustc_lint/src/context.rs

@@ -31,6 +31,7 @@ use rustc_middle::lint::LintDiagnosticBuilder;
 use rustc_middle::middle::privacy::AccessLevels;
 use rustc_middle::middle::stability;
 use rustc_middle::ty::layout::{LayoutError, TyAndLayout};
+use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self, print::Printer, subst::GenericArg, Ty, TyCtxt};
 use rustc_session::lint::{add_elided_lifetime_in_path_suggestion, BuiltinLintDiagnostics};
 use rustc_session::lint::{FutureIncompatibleInfo, Level, Lint, LintBuffer, LintId};
@@ -795,10 +796,12 @@ impl<'tcx> LateContext<'tcx> {
                 }
 
                 // This shouldn't ever be needed, but just in case:
-                Ok(vec![match trait_ref {
-                    Some(trait_ref) => Symbol::intern(&format!("{:?}", trait_ref)),
-                    None => Symbol::intern(&format!("<{}>", self_ty)),
-                }])
+                with_no_trimmed_paths(|| {
+                    Ok(vec![match trait_ref {
+                        Some(trait_ref) => Symbol::intern(&format!("{:?}", trait_ref)),
+                        None => Symbol::intern(&format!("<{}>", self_ty)),
+                    }])
+                })
             }
 
             fn path_append_impl(
@@ -812,12 +815,16 @@ impl<'tcx> LateContext<'tcx> {
 
                 // This shouldn't ever be needed, but just in case:
                 path.push(match trait_ref {
-                    Some(trait_ref) => Symbol::intern(&format!(
-                        "<impl {} for {}>",
-                        trait_ref.print_only_trait_path(),
-                        self_ty
-                    )),
-                    None => Symbol::intern(&format!("<impl {}>", self_ty)),
+                    Some(trait_ref) => with_no_trimmed_paths(|| {
+                        Symbol::intern(&format!(
+                            "<impl {} for {}>",
+                            trait_ref.print_only_trait_path(),
+                            self_ty
+                        ))
+                    }),
+                    None => {
+                        with_no_trimmed_paths(|| Symbol::intern(&format!("<impl {}>", self_ty)))
+                    }
                 });
 
                 Ok(path)

compiler/rustc_macros/src/query.rs

@@ -392,7 +392,7 @@ fn add_query_description_impl(
             #tcx: TyCtxt<'tcx>,
             #key: #arg,
         ) -> Cow<'static, str> {
-            format!(#desc).into()
+            ::rustc_middle::ty::print::with_no_trimmed_paths(|| format!(#desc).into())
         }
     };
 

compiler/rustc_middle/src/middle/stability.rs

@@ -13,6 +13,7 @@ use rustc_hir as hir;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX};
 use rustc_hir::{self, HirId};
+use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_session::lint::builtin::{DEPRECATED, DEPRECATED_IN_FUTURE, SOFT_UNSTABLE};
 use rustc_session::lint::{BuiltinLintDiagnostics, Lint, LintBuffer};
 use rustc_session::parse::feature_err_issue;
@@ -308,7 +309,7 @@ impl<'tcx> TyCtxt<'tcx> {
                 // #[rustc_deprecated] however wants to emit down the whole
                 // hierarchy.
                 if !skip || depr_entry.attr.is_since_rustc_version {
-                    let path = &self.def_path_str(def_id);
+                    let path = &with_no_trimmed_paths(|| self.def_path_str(def_id));
                     let kind = self.def_kind(def_id).descr(def_id);
                     let (message, lint) = deprecation_message(&depr_entry.attr, kind, path);
                     late_report_deprecation(

compiler/rustc_middle/src/mir/interpret/mod.rs

@@ -108,6 +108,7 @@ use rustc_data_structures::sync::{HashMapExt, Lock};
 use rustc_data_structures::tiny_list::TinyList;
 use rustc_hir::def_id::DefId;
 use rustc_macros::HashStable;
+use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_serialize::{Decodable, Encodable};
 use rustc_target::abi::{Endian, Size};
 
@@ -145,7 +146,7 @@ pub struct GlobalId<'tcx> {
 
 impl GlobalId<'tcx> {
     pub fn display(self, tcx: TyCtxt<'tcx>) -> String {
-        let instance_name = tcx.def_path_str(self.instance.def.def_id());
+        let instance_name = with_no_trimmed_paths(|| tcx.def_path_str(self.instance.def.def_id()));
         if let Some(promoted) = self.promoted {
             format!("{}::{:?}", instance_name, promoted)
         } else {

compiler/rustc_middle/src/query/mod.rs

@@ -1255,6 +1255,11 @@ rustc_queries! {
             storage(ArenaCacheSelector<'tcx>)
             desc { "calculating the visible parent map" }
         }
+        query trimmed_def_paths(_: CrateNum)
+            -> FxHashMap<DefId, Symbol> {
+            storage(ArenaCacheSelector<'tcx>)
+            desc { "calculating trimmed def paths" }
+        }
         query missing_extern_crate_item(_: CrateNum) -> bool {
             eval_always
             desc { "seeing if we're missing an `extern crate` item for this crate" }

compiler/rustc_middle/src/ty/context.rs

@@ -944,7 +944,7 @@ pub struct GlobalCtxt<'tcx> {
     maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
     /// A map of glob use to a set of names it actually imports. Currently only
     /// used in save-analysis.
-    glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,
+    pub(crate) glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,
     /// Extern prelude entries. The value is `true` if the entry was introduced
     /// via `extern crate` item and not `--extern` option or compiler built-in.
     pub extern_prelude: FxHashMap<Symbol, bool>,

compiler/rustc_middle/src/ty/instance.rs

@@ -260,10 +260,11 @@ impl<'tcx> fmt::Display for Instance<'tcx> {
             InstanceDef::ReifyShim(_) => write!(f, " - shim(reify)"),
             InstanceDef::Intrinsic(_) => write!(f, " - intrinsic"),
             InstanceDef::Virtual(_, num) => write!(f, " - virtual#{}", num),
-            InstanceDef::FnPtrShim(_, ty) => write!(f, " - shim({:?})", ty),
+            InstanceDef::FnPtrShim(_, ty) => write!(f, " - shim({})", ty),
             InstanceDef::ClosureOnceShim { .. } => write!(f, " - shim"),
-            InstanceDef::DropGlue(_, ty) => write!(f, " - shim({:?})", ty),
-            InstanceDef::CloneShim(_, ty) => write!(f, " - shim({:?})", ty),
+            InstanceDef::DropGlue(_, None) => write!(f, " - shim(None)"),
+            InstanceDef::DropGlue(_, Some(ty)) => write!(f, " - shim(Some({}))", ty),
+            InstanceDef::CloneShim(_, ty) => write!(f, " - shim({})", ty),
         }
     }
 }