Skip to content

diagnostics: shorten paths of unique symbols #73996

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

Merged
merged 6 commits into from
Sep 4, 2020
Merged
Changes from all 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
@@ -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)
{
19 changes: 11 additions & 8 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
@@ -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();

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

55 changes: 48 additions & 7 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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()));
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));
}
@@ -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)) {
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
Original file line number Diff line number Diff line change
@@ -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,
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
@@ -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);
5 changes: 4 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
@@ -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 {}",
27 changes: 17 additions & 10 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 1 addition & 1 deletion compiler/rustc_macros/src/query.rs
Original file line number Diff line number Diff line change
@@ -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())
}
};

3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/middle/stability.rs
Original file line number Diff line number Diff line change
@@ -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(
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
@@ -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 {
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
@@ -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" }
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
@@ -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>,
7 changes: 4 additions & 3 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
@@ -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),
}
}
}
Loading