-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Track span of function in method calls, and use this in #[track_caller] #73182
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
Changes from all commits
28946b3
6e4d0b4
f69a2a6
bbf497b
e5f3b94
9d36fa3
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 |
|---|---|---|
|
|
@@ -1371,7 +1371,7 @@ pub struct Expr<'hir> { | |
|
|
||
| // `Expr` is used a lot. Make sure it doesn't unintentionally get bigger. | ||
| #[cfg(target_arch = "x86_64")] | ||
| rustc_data_structures::static_assert_size!(Expr<'static>, 64); | ||
| rustc_data_structures::static_assert_size!(Expr<'static>, 72); | ||
|
Contributor
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. You'll want to do a perf run for this even though it's only 8 bytes. |
||
|
|
||
| impl Expr<'_> { | ||
| pub fn precedence(&self) -> ExprPrecedence { | ||
|
|
@@ -1568,12 +1568,14 @@ pub enum ExprKind<'hir> { | |
| /// and the remaining elements are the rest of the arguments. | ||
| /// Thus, `x.foo::<Bar, Baz>(a, b, c, d)` is represented as | ||
| /// `ExprKind::MethodCall(PathSegment { foo, [Bar, Baz] }, [x, a, b, c, d])`. | ||
| /// The final `Span` represents the span of the function and arguments | ||
| /// (e.g. `foo::<Bar, Baz>(a, b, c, d)` in `x.foo::<Bar, Baz>(a, b, c, d)` | ||
| /// | ||
| /// To resolve the called method to a `DefId`, call [`type_dependent_def_id`] with | ||
| /// the `hir_id` of the `MethodCall` node itself. | ||
| /// | ||
| /// [`type_dependent_def_id`]: ../ty/struct.TypeckTables.html#method.type_dependent_def_id | ||
| MethodCall(&'hir PathSegment<'hir>, Span, &'hir [Expr<'hir>]), | ||
| MethodCall(&'hir PathSegment<'hir>, Span, &'hir [Expr<'hir>], Span), | ||
|
Contributor
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. Having a 4-field tuple variant where two fields have the same type is not great. Maybe now is the time to switch to named fields? I really wish there was an automated way of doing this.
Contributor
Author
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. I thought about that, but I wanted to avoid creating even more churn. This field is ignored everywhere exept in a few specific cases, so I don't think it's too bad.
Contributor
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 doesn't seem necessary as |
||
| /// A tuple (e.g., `(a, b, c, d)`). | ||
| Tup(&'hir [Expr<'hir>]), | ||
| /// A binary operation (e.g., `a + b`, `a * b`). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1131,6 +1131,9 @@ pub enum TerminatorKind<'tcx> { | |
| /// `true` if this is from a call in HIR rather than from an overloaded | ||
| /// operator. True for overloaded function call. | ||
| from_hir_call: bool, | ||
| /// This `Span` is the span of the function, without the dot and receiver | ||
| /// (e.g. `foo(a, b)` in `x.foo(a, b)` | ||
| fn_span: Span, | ||
|
Contributor
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. Needs a doc comment. Also, are we trying to move away from spans inside MIR data structures for incremental? Seems like
Contributor
Author
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. We're storing an almost identical span (
Contributor
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. I wonder if this should be the span in |
||
| }, | ||
|
|
||
| /// Jump to the target if the condition has the expected value, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| use std::convert::TryFrom; | ||
|
|
||
| use rustc_hir::lang_items::PanicLocationLangItem; | ||
| use rustc_middle::mir::TerminatorKind; | ||
| use rustc_middle::ty::subst::Subst; | ||
| use rustc_span::{Span, Symbol}; | ||
| use rustc_target::abi::LayoutOf; | ||
|
|
@@ -14,19 +15,32 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | |
| /// Walks up the callstack from the intrinsic's callsite, searching for the first callsite in a | ||
| /// frame which is not `#[track_caller]`. | ||
| crate fn find_closest_untracked_caller_location(&self) -> Span { | ||
| self.stack() | ||
| let frame = self | ||
| .stack() | ||
| .iter() | ||
| .rev() | ||
| // Find first non-`#[track_caller]` frame. | ||
| .find(|frame| !frame.instance.def.requires_caller_location(*self.tcx)) | ||
| .find(|frame| { | ||
| debug!( | ||
| "find_closest_untracked_caller_location: checking frame {:?}", | ||
| frame.instance | ||
| ); | ||
| !frame.instance.def.requires_caller_location(*self.tcx) | ||
| }) | ||
| // Assert that there is always such a frame. | ||
| .unwrap() | ||
| .current_source_info() | ||
| // Assert that the frame we look at is actually executing code currently | ||
| // (`current_source_info` is None when we are unwinding and the frame does | ||
| // not require cleanup). | ||
|
Member
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 comment was lost, but it still seems important. |
||
| .unwrap() | ||
| .span | ||
| .unwrap(); | ||
| let loc = frame.loc.unwrap(); | ||
| let block = &frame.body.basic_blocks()[loc.block]; | ||
| assert_eq!(block.statements.len(), loc.statement_index); | ||
| debug!( | ||
| "find_closest_untracked_caller_location:: got terminator {:?} ({:?})", | ||
| block.terminator(), | ||
| block.terminator().kind | ||
| ); | ||
| if let TerminatorKind::Call { fn_span, .. } = block.terminator().kind { | ||
| return fn_span; | ||
| } | ||
| unreachable!(); | ||
|
Member
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. I think this
Member
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.
|
||
| } | ||
|
|
||
| /// Allocate a `const core::panic::Location` with the provided filename and line/column numbers. | ||
|
|
||
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.
Is this
Spannot available as part of theIdentthat is the name of the method, inPathSegment?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.
This
Spanalso includes the opening and closing parenthesis, so thePathSegmentandVec<P<Expr>>are insufficient to reconstruct it exactly.