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

Jump to bottom

Make some diagnostics not depend on the source of what they reference being available #105500

Merged
merged 8 commits into from
Dec 14, 2022
+381 −1,666
Merged

Make some diagnostics not depend on the source of what they reference being available #105500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cb26b35
Make some diagnostics not depend on the source of what they reference…
oli-obk Dec 9, 2022
717294f
Inform the user which trait is meant in the diagnostic itself instead…
oli-obk Dec 12, 2022
063b167
Clarify what "this" means
oli-obk Dec 12, 2022
2e2a479
Don't emit empty notes
oli-obk Dec 12, 2022
59554a2
Avoid rendering empty annotations
oli-obk Dec 12, 2022
6984085
Stop pointing to operators if their libcore method source is not avai…
oli-obk Dec 12, 2022
1f5cb9e
Use a label instead of a note for the drop site to create denser diag…
oli-obk Dec 12, 2022
82ce70a
bless fulldeps tests
oli-obk Dec 12, 2022
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
24 changes: 16 additions & 8 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1059,17 +1059,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
if self.fn_self_span_reported.insert(fn_span) {
err.span_note(
// Check whether the source is accessible
if self.infcx.tcx.sess.source_map().is_span_accessible(self_arg.span) {
self_arg.span
} else {
fn_call_span
},
self_arg.span,
"calling this operator moves the left-hand side",
);
}
}
CallKind::Normal { self_arg, desugaring, is_option_or_result } => {
CallKind::Normal { self_arg, desugaring, method_did } => {
let self_arg = self_arg.unwrap();
if let Some((CallDesugaringKind::ForLoopIntoIter, _)) = desugaring {
let ty = moved_place.ty(self.body, self.infcx.tcx).ty;
Expand Down Expand Up @@ -1139,14 +1134,27 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
),
);
}
let tcx = self.infcx.tcx;
// Avoid pointing to the same function in multiple different
// error messages.
if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) {
let func = tcx.def_path_str(method_did);
err.span_note(
self_arg.span,
&format!("this function takes ownership of the receiver `self`, which moves {}", place_name)
&format!("`{func}` takes ownership of the receiver `self`, which moves {place_name}")
);
}
let parent_did = tcx.parent(method_did);
let parent_self_ty = (tcx.def_kind(parent_did)
== rustc_hir::def::DefKind::Impl)
.then_some(parent_did)
.and_then(|did| match tcx.type_of(did).kind() {
ty::Adt(def, ..) => Some(def.did()),
_ => None,
});
let is_option_or_result = parent_self_ty.map_or(false, |def_id| {
matches!(tcx.get_diagnostic_name(def_id), Some(sym::Option | sym::Result))
});
if is_option_or_result && maybe_reinitialized_locations_is_empty {
err.span_label(
var_span,
Expand Down
18 changes: 3 additions & 15 deletions compiler/rustc_const_eval/src/util/call_kind.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use rustc_hir::def_id::DefId;
use rustc_hir::{lang_items, LangItem};
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, AssocItemContainer, DefIdTree, Instance, ParamEnv, Ty, TyCtxt};
use rustc_middle::ty::{AssocItemContainer, Instance, ParamEnv, Ty, TyCtxt};
use rustc_span::symbol::Ident;
use rustc_span::{sym, DesugaringKind, Span};

Expand Down Expand Up @@ -39,9 +39,7 @@ pub enum CallKind<'tcx> {
Normal {
self_arg: Option<Ident>,
desugaring: Option<(CallDesugaringKind, Ty<'tcx>)>,
/// Whether the self type of the method call has an `.as_ref()` method.
/// Used for better diagnostics.
is_option_or_result: bool,
method_did: DefId,
},
/// A call to `Fn(..)::call(..)`, desugared from `my_closure(a, b, c)`
FnCall { fn_trait_id: DefId, self_ty: Ty<'tcx> },
Expand Down Expand Up @@ -133,16 +131,6 @@ pub fn call_kind<'tcx>(
} else {
None
};
let parent_did = tcx.parent(method_did);
let parent_self_ty = (tcx.def_kind(parent_did) == rustc_hir::def::DefKind::Impl)
.then_some(parent_did)
.and_then(|did| match tcx.type_of(did).kind() {
ty::Adt(def, ..) => Some(def.did()),
_ => None,
});
let is_option_or_result = parent_self_ty.map_or(false, |def_id| {
matches!(tcx.get_diagnostic_name(def_id), Some(sym::Option | sym::Result))
});
CallKind::Normal { self_arg, desugaring, is_option_or_result }
CallKind::Normal { self_arg, desugaring, method_did }
})
}
79 changes: 44 additions & 35 deletions compiler/rustc_errors/src/emitter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1408,49 +1408,58 @@ impl EmitterWriter {
if !sm.ensure_source_file_source_present(annotated_file.file.clone()) {
if !self.short_message {
// We'll just print an unannotated message.
for (annotation_id, line) in annotated_file.lines.into_iter().enumerate() {
for (annotation_id, line) in annotated_file.lines.iter().enumerate() {
let mut annotations = line.annotations.clone();
annotations.sort_by_key(|a| Reverse(a.start_col));
let mut line_idx = buffer.num_lines();
buffer.append(
line_idx,
&format!(
"{}:{}:{}",
sm.filename_for_diagnostics(&annotated_file.file.name),
sm.doctest_offset_line(&annotated_file.file.name, line.line_index),
annotations[0].start_col + 1,
),
Style::LineAndColumn,
);
if annotation_id == 0 {
buffer.prepend(line_idx, "--> ", Style::LineNumber);

let labels: Vec<_> = annotations
.iter()
.filter_map(|a| Some((a.label.as_ref()?, a.is_primary)))
.filter(|(l, _)| !l.is_empty())
.collect();

if annotation_id == 0 || !labels.is_empty() {
buffer.append(
line_idx,
&format!(
"{}:{}:{}",
sm.filename_for_diagnostics(&annotated_file.file.name),
sm.doctest_offset_line(
&annotated_file.file.name,
line.line_index
),
annotations[0].start_col + 1,
),
Style::LineAndColumn,
);
if annotation_id == 0 {
buffer.prepend(line_idx, "--> ", Style::LineNumber);
} else {
buffer.prepend(line_idx, "::: ", Style::LineNumber);
}
for _ in 0..max_line_num_len {
buffer.prepend(line_idx, " ", Style::NoStyle);
}
line_idx += 1;
};
for (i, annotation) in annotations.into_iter().enumerate() {
if let Some(label) = &annotation.label {
let style = if annotation.is_primary {
Style::LabelPrimary
} else {
Style::LabelSecondary
};
if annotation_id == 0 {
buffer.prepend(line_idx, " |", Style::LineNumber);
for _ in 0..max_line_num_len {
buffer.prepend(line_idx, " ", Style::NoStyle);
}
line_idx += 1;
buffer.append(line_idx + i, " = note: ", style);
for _ in 0..max_line_num_len {
buffer.prepend(line_idx, " ", Style::NoStyle);
}
} else {
buffer.append(line_idx + i, ": ", style);
}
buffer.append(line_idx + i, label, style);
}
for (label, is_primary) in labels.into_iter() {
let style = if is_primary {
Style::LabelPrimary
} else {
Style::LabelSecondary
};
buffer.prepend(line_idx, " |", Style::LineNumber);
for _ in 0..max_line_num_len {
buffer.prepend(line_idx, " ", Style::NoStyle);
}
line_idx += 1;
buffer.append(line_idx, " = note: ", style);
for _ in 0..max_line_num_len {
buffer.prepend(line_idx, " ", Style::NoStyle);
}
buffer.append(line_idx, label, style);
line_idx += 1;
}
}
}
Expand Down
23 changes: 18 additions & 5 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 @@ -1853,7 +1853,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)],
) {
let mut derives = Vec::<(String, Span, Symbol)>::new();
let mut traits = Vec::<Span>::new();
let mut traits = Vec::new();
for (pred, _, _) in unsatisfied_predicates {
let ty::PredicateKind::Clause(ty::Clause::Trait(trait_pred)) = pred.kind().skip_binder() else { continue };
let adt = match trait_pred.self_ty().ty_adt_def() {
Expand Down Expand Up @@ -1892,10 +1892,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
derives.push((self_name, self_span, diagnostic_name));
} else {
traits.push(self.tcx.def_span(trait_pred.def_id()));
traits.push(trait_pred.def_id());
}
} else {
traits.push(self.tcx.def_span(trait_pred.def_id()));
traits.push(trait_pred.def_id());
}
}
traits.sort();
Expand All @@ -1918,10 +1918,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let len = traits.len();
if len > 0 {
let span: MultiSpan = traits.into();
let span =
MultiSpan::from_spans(traits.iter().map(|&did| self.tcx.def_span(did)).collect());
let mut names = format!("`{}`", self.tcx.def_path_str(traits[0]));
for (i, &did) in traits.iter().enumerate().skip(1) {
if len > 2 {
names.push_str(", ");
}
if i == len - 1 {
names.push_str(" and ");
}
names.push('`');
names.push_str(&self.tcx.def_path_str(did));
names.push('`');
}
err.span_note(
span,
&format!("the following trait{} must be implemented", pluralize!(len),),
&format!("the trait{} {} must be implemented", pluralize!(len), names),
);
}

Expand Down
16 changes: 9 additions & 7 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1527,13 +1527,15 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
if let Some(virtual_dir) = &sess.opts.unstable_opts.simulate_remapped_rust_src_base
{
if let Some(real_dir) = &sess.opts.real_rust_source_base_dir {
if let rustc_span::FileName::Real(ref mut old_name) = name {
if let rustc_span::RealFileName::LocalPath(local) = old_name {
if let Ok(rest) = local.strip_prefix(real_dir) {
*old_name = rustc_span::RealFileName::Remapped {
local_path: None,
virtual_name: virtual_dir.join(rest),
};
for subdir in ["library", "compiler"] {
if let rustc_span::FileName::Real(ref mut old_name) = name {
if let rustc_span::RealFileName::LocalPath(local) = old_name {
if let Ok(rest) = local.strip_prefix(real_dir.join(subdir)) {
*old_name = rustc_span::RealFileName::Remapped {
local_path: None,
virtual_name: virtual_dir.join(subdir).join(rest),
};
}
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_privacy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,15 +1308,15 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> {
let is_local_static =
if let DefKind::Static(_) = kind { def_id.is_local() } else { false };
if !self.item_is_accessible(def_id) && !is_local_static {
let sess = self.tcx.sess;
let sm = sess.source_map();
let name = match qpath {
hir::QPath::Resolved(..) | hir::QPath::LangItem(..) => {
sm.span_to_snippet(qpath.span()).ok()
let name = match *qpath {
hir::QPath::LangItem(it, ..) => {
self.tcx.lang_items().get(it).map(|did| self.tcx.def_path_str(did))
}
hir::QPath::Resolved(_, path) => Some(self.tcx.def_path_str(path.res.def_id())),
hir::QPath::TypeRelative(_, segment) => Some(segment.ident.to_string()),
};
let kind = kind.descr(def_id);
let sess = self.tcx.sess;
let _ = match name {
Some(name) => {
sess.emit_err(ItemIsPrivate { span, kind, descr: (&name).into() })
Expand Down
32 changes: 11 additions & 21 deletions compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2179,15 +2179,15 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
format!("does not implement `{}`", trait_pred.print_modifiers_and_trait_path())
};

let mut explain_yield = |interior_span: Span,
yield_span: Span,
scope_span: Option<Span>| {
let mut span = MultiSpan::from_span(yield_span);
if let Ok(snippet) = source_map.span_to_snippet(interior_span) {
// #70935: If snippet contains newlines, display "the value" instead
// so that we do not emit complex diagnostics.
let snippet = &format!("`{}`", snippet);
let snippet = if snippet.contains('\n') { "the value" } else { snippet };
let mut explain_yield =
|interior_span: Span, yield_span: Span, scope_span: Option<Span>| {
let mut span = MultiSpan::from_span(yield_span);
let snippet = match source_map.span_to_snippet(interior_span) {
// #70935: If snippet contains newlines, display "the value" instead
// so that we do not emit complex diagnostics.
Ok(snippet) if !snippet.contains('\n') => format!("`{}`", snippet),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a length check here too, just to make sure that the snippet is not for a long expr in one line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe this entire diagnostic deserves a redesign to point at the span instead of getting the snippet in a message?

_ => "the value".to_string(),
};
// note: future is not `Send` as this value is used across an await
// --> $DIR/issue-70935-complex-spans.rs:13:9
// |
Expand All @@ -2212,17 +2212,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
interior_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);
// If available, use the scope span to annotate the drop location.
let mut scope_note = None;
if let Some(scope_span) = scope_span {
let scope_span = source_map.end_point(scope_span);

let msg = format!("{} is later dropped here", snippet);
if source_map.is_multiline(yield_span.between(scope_span)) {
span.push_span_label(scope_span, msg);
} else {
scope_note = Some((scope_span, msg));
}
span.push_span_label(scope_span, msg);
}
err.span_note(
span,
Expand All @@ -2231,11 +2225,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
future_or_generator, trait_explanation, an_await_or_yield
),
);
if let Some((span, msg)) = scope_note {
err.span_note(span, &msg);
}
}
};
};
match interior_or_upvar_span {
GeneratorInteriorOrUpvar::Interior(interior_span, interior_extra_info) => {
if let Some((scope_span, yield_span, expr, from_awaited_ty)) = interior_extra_info {
Expand Down
5 changes: 1 addition & 4 deletions src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,10 +660,7 @@ LL | #[derive(Diagnostic)]
= help: normalized in stderr
note: required by a bound in `DiagnosticBuilder::<'a, G>::set_arg`
--> $COMPILER_DIR/rustc_errors/src/diagnostic_builder.rs:LL:CC
|
LL | arg: impl IntoDiagnosticArg,
| ^^^^^^^^^^^^^^^^^ required by this bound in `DiagnosticBuilder::<'a, G>::set_arg`
= note: this error originates in the derive macro `Diagnostic` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the derive macro `Diagnostic` which comes from the expansion of the macro `forward` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 83 previous errors

Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/alloc-error/alloc-error-handler-bad-signature-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ LL | | }
= note: struct `core::alloc::Layout` and struct `Layout` have similar names, but are actually distinct types
note: struct `core::alloc::Layout` is defined in crate `core`
--> $SRC_DIR/core/src/alloc/layout.rs:LL:COL
|
LL | pub struct Layout {
| ^^^^^^^^^^^^^^^^^
estebank marked this conversation as resolved.
Show resolved Hide resolved
note: struct `Layout` is defined in the current crate
--> $DIR/alloc-error-handler-bad-signature-2.rs:7:1
|
Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/associated-type-bounds/issue-99828.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ LL | fn get_iter(vec: &[i32]) -> impl Iterator<Item = {}> + '_ {
|
note: associated type defined here
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
|
LL | type Item;
| ^^^^^^^^^

error: aborting due to 2 previous errors

Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/associated-types/defaults-wf.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ LL | type Ty = Vec<[u8]>;
= help: the trait `Sized` is not implemented for `[u8]`
note: required by a bound in `Vec`
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
|
LL | pub struct Vec<T, #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global> {
| ^ required by this bound in `Vec`

error: aborting due to previous error

Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/associated-types/trait-with-supertraits-needing-sized-self.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ LL | trait ArithmeticOps: Add<Output=Self> + Sub<Output=Self> + Mul<Output=Self>
|
note: required by a bound in `Add`
--> $SRC_DIR/core/src/ops/arith.rs:LL:COL
|
LL | pub trait Add<Rhs = Self> {
| ^^^^^^^^^^ required by this bound in `Add`
help: consider further restricting `Self`
|
LL | trait ArithmeticOps: Add<Output=Self> + Sub<Output=Self> + Mul<Output=Self> + Div<Output=Self> + Sized {}
Expand Down
Loading