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.

Sign up for GitHub

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

Rework "long type names" printing logic #136328

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+320 −740
Open

Rework "long type names" printing logic #136328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions compiler/rustc_borrowck/messages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ borrowck_lifetime_constraints_error =
borrowck_limitations_implies_static =
due to current limitations in the borrow checker, this implies a `'static` lifetime
borrowck_long_type_consider_verbose = consider using `--verbose` to print the full type name to the console
borrowck_long_type_full_path = the full type name has been written to '{$path}'
borrowck_move_closure_suggestion =
consider adding 'move' keyword before the nested closure
Expand Down
8 changes: 1 addition & 7 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
None => "value".to_owned(),
};
if needs_note {
let mut path = None;
let ty = self.infcx.tcx.short_ty_string(ty, &mut path);
let ty = self.infcx.tcx.short_string(ty, err.long_ty_path());
if let Some(local) = place.as_local() {
let span = self.body.local_decls[local].source_info.span;
err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
Expand All @@ -306,11 +305,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
place: &note_msg,
});
};
if let Some(path) = path {
err.subdiagnostic(crate::session_diagnostics::LongTypePath {
path: path.display().to_string(),
});
}
}

if let UseSpans::FnSelfUse {
Expand Down
24 changes: 3 additions & 21 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,19 +596,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
self.suggest_cloning(err, place_ty, expr, None);
}

let mut path = None;
let ty = self.infcx.tcx.short_ty_string(place_ty, &mut path);
let ty = self.infcx.tcx.short_string(place_ty, err.long_ty_path());
err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
is_partial_move: false,
ty,
place: &place_desc,
span,
});
if let Some(path) = path {
err.subdiagnostic(crate::session_diagnostics::LongTypePath {
path: path.display().to_string(),
});
}
} else {
binds_to.sort();
binds_to.dedup();
Expand All @@ -635,19 +629,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
self.suggest_cloning(err, place_ty, expr, Some(use_spans));
}

let mut path = None;
let ty = self.infcx.tcx.short_ty_string(place_ty, &mut path);
let ty = self.infcx.tcx.short_string(place_ty, err.long_ty_path());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an example of the intended way of using this. This is the safest way of creating a short string because there's no way of forgetting to print the "full name written to path" message.

err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
is_partial_move: false,
ty,
place: &place_desc,
span: use_span,
});
if let Some(path) = path {
err.subdiagnostic(crate::session_diagnostics::LongTypePath {
path: path.display().to_string(),
});
}

use_spans.args_subdiag(err, |args_span| {
crate::session_diagnostics::CaptureArgLabel::MoveOutPlace {
Expand Down Expand Up @@ -845,19 +833,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
self.suggest_cloning(err, bind_to.ty, expr, None);
}

let mut path = None;
let ty = self.infcx.tcx.short_ty_string(bind_to.ty, &mut path);
let ty = self.infcx.tcx.short_string(bind_to.ty, err.long_ty_path());
err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
is_partial_move: false,
ty,
place: place_desc,
span: binding_span,
});
if let Some(path) = path {
err.subdiagnostic(crate::session_diagnostics::LongTypePath {
path: path.display().to_string(),
});
}
}
}

Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_borrowck/src/session_diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,13 +458,6 @@ pub(crate) enum OnClosureNote<'a> {
},
}

#[derive(Subdiagnostic)]
#[note(borrowck_long_type_full_path)]
#[note(borrowck_long_type_consider_verbose)]
pub(crate) struct LongTypePath {
pub(crate) path: String,
}

#[derive(Subdiagnostic)]
pub(crate) enum TypeNoCopy<'a> {
#[label(borrowck_ty_no_impl_copy)]
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_errors/src/diagnostic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::hash::{Hash, Hasher};
use std::marker::PhantomData;
use std::ops::{Deref, DerefMut};
use std::panic;
use std::path::PathBuf;
use std::thread::panicking;

use rustc_data_structures::fx::FxIndexMap;
Expand Down Expand Up @@ -301,6 +302,7 @@ pub struct DiagInner {

pub is_lint: Option<IsLint>,

pub long_ty_path: Option<PathBuf>,
/// With `-Ztrack_diagnostics` enabled,
/// we print where in rustc this error was emitted.
pub(crate) emitted_at: DiagLocation,
Expand All @@ -324,6 +326,7 @@ impl DiagInner {
args: Default::default(),
sort_span: DUMMY_SP,
is_lint: None,
long_ty_path: None,
emitted_at: DiagLocation::caller(),
}
}
Expand Down Expand Up @@ -1293,9 +1296,20 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
/// `cancel`, etc. Afterwards, `drop` is the only code that will be run on
/// `self`.
fn take_diag(&mut self) -> DiagInner {
if let Some(path) = &self.long_ty_path {
self.note(format!(
"the full name for the type has been written to '{}'",
path.display()
Copy link
Member

Choose a reason for hiding this comment

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

Question: not necessarily for this PR, but this doesn't handle path-remapping (--remap-path-prefix or -Zremap-path-scope=diagnostics and such), right?

));
self.note("consider using `--verbose` to print the full type name to the console");
}
Box::into_inner(self.diag.take().unwrap())
}
Comment on lines 1298 to 1307
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now there's no chance of forgetting to print this path (although we need to remember to use long_ty_path() to set the path right after error construction if we've made short strings before that).

Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: could you add that remark as a doc comment for long_ty_path immediately below?


pub fn long_ty_path(&mut self) -> &mut Option<PathBuf> {
&mut self.long_ty_path
}

/// Most `emit_producing_guarantee` functions use this as a starting point.
fn emit_producing_nothing(mut self) {
let diag = self.take_diag();
Expand Down
39 changes: 14 additions & 25 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use core::ops::ControlFlow;
use std::borrow::Cow;
use std::path::PathBuf;

use hir::Expr;
use rustc_ast::ast::Mutability;
Expand Down Expand Up @@ -362,14 +363,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

fn suggest_missing_writer(&self, rcvr_ty: Ty<'tcx>, rcvr_expr: &hir::Expr<'tcx>) -> Diag<'_> {
let mut file = None;
let ty_str = self.tcx.short_ty_string(rcvr_ty, &mut file);
let mut err = struct_span_code_err!(
self.dcx(),
rcvr_expr.span,
E0599,
"cannot write into `{}`",
ty_str
self.tcx.short_string(rcvr_ty, &mut file),
);
*err.long_ty_path() = file;
err.span_note(
rcvr_expr.span,
"must implement `io::Write`, `fmt::Write`, or have a `write_fmt` method",
Expand All @@ -380,11 +381,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"a writer is needed before this format string",
);
};
if let Some(file) = file {
err.note(format!("the full type name has been written to '{}'", file.display()));
err.note("consider using `--verbose` to print the full type name to the console");
}

err
}

Expand Down Expand Up @@ -595,7 +591,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(predicates.to_string(), with_forced_trimmed_paths!(predicates.to_string()))
} else {
(
tcx.short_ty_string(rcvr_ty, &mut ty_file),
tcx.short_string(rcvr_ty, &mut ty_file),
with_forced_trimmed_paths!(rcvr_ty.to_string()),
)
};
Expand Down Expand Up @@ -624,6 +620,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span,
item_name,
&short_ty_str,
&mut ty_file,
) {
return guar;
}
Expand All @@ -635,6 +632,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
item_kind,
item_name,
&short_ty_str,
&mut ty_file,
) {
return guar;
}
Expand Down Expand Up @@ -728,10 +726,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty_str = short_ty_str;
}

if let Some(file) = ty_file {
err.note(format!("the full type name has been written to '{}'", file.display(),));
err.note("consider using `--verbose` to print the full type name to the console");
}
if rcvr_ty.references_error() {
err.downgrade_to_delayed_bug();
}
Expand Down Expand Up @@ -1314,7 +1308,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
bound_list.into_iter().map(|(_, path)| path).collect::<Vec<_>>().join("\n");
let actual_prefix = rcvr_ty.prefix_string(self.tcx);
info!("unimplemented_traits.len() == {}", unimplemented_traits.len());
let mut long_ty_file = None;
let (primary_message, label, notes) = if unimplemented_traits.len() == 1
&& unimplemented_traits_only
{
Expand All @@ -1329,7 +1322,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
let OnUnimplementedNote { message, label, notes, .. } = self
.err_ctxt()
.on_unimplemented_note(trait_ref, &obligation, &mut long_ty_file);
.on_unimplemented_note(trait_ref, &obligation, &mut ty_file);
(message, label, notes)
})
.unwrap()
Expand All @@ -1343,15 +1336,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
});
err.primary_message(primary_message);
if let Some(file) = long_ty_file {
err.note(format!(
"the full name for the type has been written to '{}'",
file.display(),
));
err.note(
"consider using `--verbose` to print the full type name to the console",
);
}
if let Some(label) = label {
custom_span_label = true;
err.span_label(span, label);
Expand Down Expand Up @@ -2403,6 +2387,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span: Span,
item_name: Ident,
ty_str: &str,
long_ty_path: &mut Option<PathBuf>,
) -> Result<(), ErrorGuaranteed> {
if let SelfSource::MethodCall(expr) = source {
for (_, parent) in tcx.hir().parent_iter(expr.hir_id).take(5) {
Expand Down Expand Up @@ -2460,7 +2445,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
if pick.is_ok() {
let range_span = parent_expr.span.with_hi(expr.span.hi());
return Err(self.dcx().emit_err(errors::MissingParenthesesInRange {
let mut err = self.dcx().create_err(errors::MissingParenthesesInRange {
span,
ty_str: ty_str.to_string(),
method_name: item_name.as_str().to_string(),
Expand All @@ -2469,7 +2454,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
left: range_span.shrink_to_lo(),
right: range_span.shrink_to_hi(),
}),
}));
});
*err.long_ty_path() = long_ty_path.take();
return Err(err.emit());
}
}
}
Expand All @@ -2486,6 +2473,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
item_kind: &str,
item_name: Ident,
ty_str: &str,
long_ty_path: &mut Option<PathBuf>,
) -> Result<(), ErrorGuaranteed> {
let found_candidate = all_traits(self.tcx)
.into_iter()
Expand Down Expand Up @@ -2526,6 +2514,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
item_name,
ty_str
);
*err.long_ty_path() = long_ty_path.take();
let concrete_type = if actual.is_integral() { "i32" } else { "f32" };
match expr.kind {
ExprKind::Lit(lit) => {
Expand Down
Loading
Loading