-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Add tracing to resolve-related functions #144727
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ use super::{ | |
Projectable, Provenance, ReturnAction, ReturnContinuation, Scalar, StackPopInfo, interp_ok, | ||
throw_ub, throw_ub_custom, throw_unsup_format, | ||
}; | ||
use crate::fluent_generated as fluent; | ||
use crate::{enter_trace_span, fluent_generated as fluent}; | ||
|
||
/// An argument passed to a function. | ||
#[derive(Clone, Debug)] | ||
|
@@ -730,6 +730,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |
let existential_trait_ref = ty::ExistentialTraitRef::erase_self_ty(tcx, virtual_trait_ref); | ||
let concrete_trait_ref = existential_trait_ref.with_self_ty(tcx, dyn_ty); | ||
|
||
let _span = enter_trace_span!(M, resolve::expect_resolve_for_vtable, ?def_id); | ||
let concrete_method = Instance::expect_resolve_for_vtable( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use the same pattern with a nested block as everywhere else. |
||
tcx, | ||
self.typing_env, | ||
|
@@ -819,7 +820,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |
place | ||
} | ||
}; | ||
let instance = ty::Instance::resolve_drop_in_place(*self.tcx, place.layout.ty); | ||
let instance = { | ||
let _span = enter_trace_span!(M, resolve::resolve_drop_in_place, ty = ?place.layout.ty); | ||
ty::Instance::resolve_drop_in_place(*self.tcx, place.layout.ty) | ||
}; | ||
let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty())?; | ||
|
||
let arg = self.mplace_to_ref(&place)?; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,8 @@ pub enum AccessKind { | |
/// | ||
/// A `None` namespace indicates we are looking for a module. | ||
fn try_resolve_did(tcx: TyCtxt<'_>, path: &[&str], namespace: Option<Namespace>) -> Option<DefId> { | ||
let _span = enter_trace_span!(resolve::try_resolve_did, ?path); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't the same kind of operation as the others at all, it is using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I will call it just "try_resolve_did" then. I thought this belonged to the same category because it seemed to resolve instances from module paths. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah but the the resolves resolve instances from DefIds (by doing trait solving)... whereas this here produces a DefId (by doing basically name resolution). It's entirely different machinery. |
||
|
||
/// Yield all children of the given item, that have the given name. | ||
fn find_children<'tcx: 'a, 'a>( | ||
tcx: TyCtxt<'tcx>, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we call this
_span
everywhere? I now realize that's confusing because usually that refers to theSpan
type...Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
_s
would be better? The name is not relevant anyway, we just need it so that the returned value is not immediately destructedOr
_ets
also. I would avoid longer names (e.g._entered_trace_span
) as it'd make all tracing calls use multiple linesIn case I will open a new PR to change all of the names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have suggested
_trace
.