Skip to content

Commit

Permalink
Rollup merge of rust-lang#97805 - coolreader18:trace-suggestions, r=o…
Browse files Browse the repository at this point in the history 
…li-obk

Add proper tracing spans to rustc_trait_selection::traits::error_reporting

While I was trying to figure out rust-lang#97704 I did some of this to make the logs more legible, so I figured I'd do the whole module and open a PR with it. afaict this is an ongoing process in the compiler from the log->tracing transition? but lmk if there was a reason for the more verbose forms of logging as they are.

Also, for some of the functions with only one log in them, I put the function name as a message for that log instead of `#[instrument]`-ing the whole function with a span? but maybe the latter would actually be preferable, I'm not actually sure.
  • Loading branch information
@JohnTitor
JohnTitor authored Jun 21, 2022
2 parents b20aff2 + 976336d commit 9c800ec
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 58 deletions.
28 changes: 11 additions & 17 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let cycle = self.resolve_vars_if_possible(cycle.to_owned());
assert!(!cycle.is_empty());

debug!("report_overflow_error_cycle: cycle={:?}", cycle);
debug!(?cycle, "report_overflow_error_cycle");

// The 'deepest' obligation is most likely to have a useful
// cause 'backtrace'
Expand Down Expand Up @@ -1513,6 +1513,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
}
}

#[instrument(level = "debug", skip_all)]
fn report_projection_error(
&self,
obligation: &PredicateObligation<'tcx>,
Expand Down Expand Up @@ -1551,15 +1552,9 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
&mut obligations,
);

debug!(
"report_projection_error obligation.cause={:?} obligation.param_env={:?}",
obligation.cause, obligation.param_env
);
debug!(?obligation.cause, ?obligation.param_env);

debug!(
"report_projection_error normalized_ty={:?} data.ty={:?}",
normalized_ty, data.term,
);
debug!(?normalized_ty, data.ty = ?data.term);

let is_normalized_ty_expected = !matches!(
obligation.cause.code().peel_derives(),
Expand Down Expand Up @@ -2346,6 +2341,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
}
}

#[instrument(level = "debug", skip_all)]
fn suggest_unsized_bound_if_applicable(
&self,
err: &mut Diagnostic,
Expand All @@ -2360,10 +2356,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
) else {
return;
};
debug!(
"suggest_unsized_bound_if_applicable: pred={:?} item_def_id={:?} span={:?}",
pred, item_def_id, span
);
debug!(?pred, ?item_def_id, ?span);

let (Some(node), true) = (
self.tcx.hir().get_if_local(item_def_id),
Expand All @@ -2374,6 +2367,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
self.maybe_suggest_unsized_generics(err, span, node);
}

#[instrument(level = "debug", skip_all)]
fn maybe_suggest_unsized_generics<'hir>(
&self,
err: &mut Diagnostic,
Expand All @@ -2384,8 +2378,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
return;
};
let sized_trait = self.tcx.lang_items().sized_trait();
debug!("maybe_suggest_unsized_generics: generics.params={:?}", generics.params);
debug!("maybe_suggest_unsized_generics: generics.predicates={:?}", generics.predicates);
debug!(?generics.params);
debug!(?generics.predicates);
let Some(param) = generics.params.iter().find(|param| param.span == span) else {
return;
};
Expand All @@ -2399,7 +2393,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
if explicitly_sized {
return;
}
debug!("maybe_suggest_unsized_generics: param={:?}", param);
debug!(?param);
match node {
hir::Node::Item(
item @ hir::Item {
Expand Down Expand Up @@ -2517,7 +2511,7 @@ impl<'v> Visitor<'v> for FindTypeParam {
if path.segments.len() == 1 && path.segments[0].ident.name == self.param =>
{
if !self.nested {
debug!("FindTypeParam::visit_ty: ty={:?}", ty);
debug!(?ty, "FindTypeParam::visit_ty");
self.invalid_spans.push(ty.span);
}
}
Expand Down
66 changes: 25 additions & 41 deletions compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1628,16 +1628,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
/// ```
///
/// Returns `true` if an async-await specific note was added to the diagnostic.
#[instrument(level = "debug", skip_all, fields(?obligation.predicate, ?obligation.cause.span))]
fn maybe_note_obligation_cause_for_async_await(
&self,
err: &mut Diagnostic,
obligation: &PredicateObligation<'tcx>,
) -> bool {
debug!(
"maybe_note_obligation_cause_for_async_await: obligation.predicate={:?} \
obligation.cause.span={:?}",
obligation.predicate, obligation.cause.span
);
let hir = self.tcx.hir();

// Attempt to detect an async-await error by looking at the obligation causes, looking
Expand Down Expand Up @@ -1677,18 +1673,17 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let mut seen_upvar_tys_infer_tuple = false;

while let Some(code) = next_code {
debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code);
debug!(?code);
match code {
ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => {
next_code = Some(parent_code);
}
ObligationCauseCode::ImplDerivedObligation(cause) => {
let ty = cause.derived.parent_trait_pred.skip_binder().self_ty();
debug!(
"maybe_note_obligation_cause_for_async_await: ImplDerived \
parent_trait_ref={:?} self_ty.kind={:?}",
cause.derived.parent_trait_pred,
ty.kind()
parent_trait_ref = ?cause.derived.parent_trait_pred,
self_ty.kind = ?ty.kind(),
"ImplDerived",
);

match *ty.kind() {
Expand Down Expand Up @@ -1717,10 +1712,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
| ObligationCauseCode::BuiltinDerivedObligation(derived_obligation) => {
let ty = derived_obligation.parent_trait_pred.skip_binder().self_ty();
debug!(
"maybe_note_obligation_cause_for_async_await: \
parent_trait_ref={:?} self_ty.kind={:?}",
derived_obligation.parent_trait_pred,
ty.kind()
parent_trait_ref = ?derived_obligation.parent_trait_pred,
self_ty.kind = ?ty.kind(),
);

match *ty.kind() {
Expand Down Expand Up @@ -1750,7 +1743,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}

// Only continue if a generator was found.
debug!(?generator, ?trait_ref, ?target_ty, "maybe_note_obligation_cause_for_async_await");
debug!(?generator, ?trait_ref, ?target_ty);
let (Some(generator_did), Some(trait_ref), Some(target_ty)) = (generator, trait_ref, target_ty) else {
return false;
};
Expand All @@ -1760,12 +1753,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let in_progress_typeck_results = self.in_progress_typeck_results.map(|t| t.borrow());
let generator_did_root = self.tcx.typeck_root_def_id(generator_did);
debug!(
"maybe_note_obligation_cause_for_async_await: generator_did={:?} \
generator_did_root={:?} in_progress_typeck_results.hir_owner={:?} span={:?}",
generator_did,
generator_did_root,
in_progress_typeck_results.as_ref().map(|t| t.hir_owner),
span
?generator_did,
?generator_did_root,
in_progress_typeck_results.hir_owner = ?in_progress_typeck_results.as_ref().map(|t| t.hir_owner),
?span,
);

let generator_body = generator_did
Expand All @@ -1788,7 +1779,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
if let Some(body) = generator_body {
visitor.visit_body(body);
}
debug!("maybe_note_obligation_cause_for_async_await: awaits = {:?}", visitor.awaits);
debug!(awaits = ?visitor.awaits);

// Look for a type inside the generator interior that matches the target type to get
// a span.
Expand All @@ -1809,11 +1800,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let ty_erased = self.tcx.erase_late_bound_regions(ty);
let ty_erased = self.tcx.erase_regions(ty_erased);
let eq = ty_erased == target_ty_erased;
debug!(
"maybe_note_obligation_cause_for_async_await: ty_erased={:?} \
target_ty_erased={:?} eq={:?}",
ty_erased, target_ty_erased, eq
);
debug!(?ty_erased, ?target_ty_erased, ?eq);
eq
};

Expand Down Expand Up @@ -1888,6 +1875,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {

/// Unconditionally adds the diagnostic note described in
/// `maybe_note_obligation_cause_for_async_await`'s documentation comment.
#[instrument(level = "debug", skip_all)]
fn note_obligation_cause_for_async_await(
&self,
err: &mut Diagnostic,
Expand Down Expand Up @@ -2037,8 +2025,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
} else {
// Look at the last interior type to get a span for the `.await`.
debug!(
"note_obligation_cause_for_async_await generator_interior_types: {:#?}",
typeck_results.as_ref().map(|t| &t.generator_interior_types)
generator_interior_types = ?format_args!(
"{:#?}", typeck_results.as_ref().map(|t| &t.generator_interior_types)
),
);
explain_yield(interior_span, yield_span, scope_span);
}
Expand Down Expand Up @@ -2073,7 +2062,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// bar(Foo(std::ptr::null())).await;
// ^^^^^^^^^^^^^^^^^^^^^ raw-ptr `*T` created inside this struct ctor.
// ```
debug!("parent_def_kind: {:?}", self.tcx.def_kind(parent_did));
debug!(parent_def_kind = ?self.tcx.def_kind(parent_did));
let is_raw_borrow_inside_fn_like_call =
match self.tcx.def_kind(parent_did) {
DefKind::Fn | DefKind::Ctor(..) => target_ty.is_unsafe_ptr(),
Expand Down Expand Up @@ -2131,7 +2120,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {

// Add a note for the item obligation that remains - normally a note pointing to the
// bound that introduced the obligation (e.g. `T: Send`).
debug!("note_obligation_cause_for_async_await: next_code={:?}", next_code);
debug!(?next_code);
self.note_obligation_cause_code(
err,
&obligation.predicate,
Expand Down Expand Up @@ -2688,20 +2677,16 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
));
}

#[instrument(
level = "debug", skip(self, err), fields(trait_pred.self_ty = ?trait_pred.self_ty())
)]
fn suggest_await_before_try(
&self,
err: &mut Diagnostic,
obligation: &PredicateObligation<'tcx>,
trait_pred: ty::PolyTraitPredicate<'tcx>,
span: Span,
) {
debug!(
"suggest_await_before_try: obligation={:?}, span={:?}, trait_pred={:?}, trait_pred_self_ty={:?}",
obligation,
span,
trait_pred,
trait_pred.self_ty()
);
let body_hir_id = obligation.cause.body_id;
let item_id = self.tcx.hir().get_parent_node(body_hir_id);

Expand Down Expand Up @@ -2739,14 +2724,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
);

debug!(
"suggest_await_before_try: normalized_projection_type {:?}",
self.resolve_vars_if_possible(projection_ty)
normalized_projection_type = ?self.resolve_vars_if_possible(projection_ty)
);
let try_obligation = self.mk_trait_obligation_with_new_self_ty(
obligation.param_env,
trait_pred.map_bound(|trait_pred| (trait_pred, projection_ty.skip_binder())),
);
debug!("suggest_await_before_try: try_trait_obligation {:?}", try_obligation);
debug!(try_trait_obligation = ?try_obligation);
if self.predicate_may_hold(&try_obligation)
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span)
&& snippet.ends_with('?')
Expand Down

0 comments on commit 9c800ec

Please sign in to comment.