Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

diagnostics: shorten paths of unique symbols #73996

Merged
merged 6 commits into from
Sep 4, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)
{
Expand Down
19 changes: 11 additions & 8 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,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};
Expand Down Expand Up @@ -127,6 +127,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;
}
}

Expand Down
55 changes: 48 additions & 7 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/")]
#![feature(crate_visibility_modifier)]
#![feature(backtrace)]
#![feature(nll)]

#[macro_use]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
);
}
}
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -768,6 +781,8 @@ impl HandlerInner {
}
if diagnostic.is_error() {
self.bump_err_count();
} else {
self.bump_warn_count();
}
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might need to account for formatting, especially around newlines. But also, is this necessary? If you can force the above emit_diagnostic to run, wouldn't that give you a backtrace (and a query stack, etc.)? Or is this for diagnosing issues on CI?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the backtrace here is to capture the full call site, but to not print anything, because it is not yet an error, so forcing the call to emit_diagnostic would not serve this purpose. It popped in CI when I was working on the PR and the formatting worked out alright.

self.delayed_good_path_bugs.push(diagnostic);
}

fn failure(&mut self, msg: &str) {
self.emit_diagnostic(&Diagnostic::new(FailureNote, msg));
}
Expand Down Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 17 additions & 10 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_macros/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
};

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -943,7 +943,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>,
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ pub enum LayoutError<'tcx> {
impl<'tcx> fmt::Display for LayoutError<'tcx> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match *self {
LayoutError::Unknown(ty) => write!(f, "the type `{:?}` has an unknown layout", ty),
LayoutError::Unknown(ty) => write!(f, "the type `{}` has an unknown layout", ty),
LayoutError::SizeOverflow(ty) => {
write!(f, "the type `{:?}` is too big for the current architecture", ty)
write!(f, "the type `{}` is too big for the current architecture", ty)
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3101,6 +3101,7 @@ pub fn provide(providers: &mut ty::query::Providers) {
erase_regions::provide(providers);
layout::provide(providers);
util::provide(providers);
print::provide(providers);
super::util::bug::provide(providers);
*providers = ty::query::Providers {
trait_impls_of: trait_def::trait_impls_of_provider,
Expand Down
Loading