-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Description
This is a tracking issue for the RFC "Implicit caller location" (rust-lang/rfcs#2091).
Steps:
- Implement the RFC (cc @rust-lang/compiler -- can anyone write up mentoring instructions?)Adjust documentation ()Stabilization PR ()To pick up a draggable item, press the space bar. While dragging, use the arrow keys to move the item. Press space again to drop the item in its new position, or press escape to cancel.
Unresolved questions:
-
If we want to support adding#[track_caller]
to trait methods, the redirection
pass/query/whatever should be placed after monomorphization, not before. Currently the RFC
simply prohibits applying#[track_caller]
to trait methods as a future-proofing measure. -
Diverging functions should be supported. -
The closurefoo::{{closure}}
should inherit most attributes applied to the functionfoo
, in
particular#[inline]
,#[cold]
,#[naked]
and also the ABI. Currently a procedural macro
won't see any of these, nor would there be anyway to apply these attributes to a closure.
Therefore,#[rustc_implicit_caller_location]
currently will reject#[naked]
and ABI, and
leaving#[inline]
and#[cold]
mean no-op. There is no semantic reason why these cannot be
used though.
Activity
eddyb commentedon Jan 27, 2018
This refers to
impl
of atrait
, not the declarations within thetrait
, right?IMO the easiest way to implement this involves no procedural macros or MIR passes.
We'd just pass extra arguments on direct calls, and require a shim to get function pointers of the function. That would also surely work with trait methods, since it'd be post-monomorphization.
To explain, the shim (which we may already have some variation of) would "just" do the direct call to the function, which would pass the location of the shim, the same as the original function.
nikomatsakis commentedon Jan 29, 2018
I can't remember. =) But I suspect so. I don't think there's anything special about "trait methods" per se -- it's really about dynamic dispatch.
nikomatsakis commentedon Apr 25, 2018
@kennytm had a prototype implementation:
https://github.com/kennytm/rust/tree/caller-info-4
Maybe @kennytm you can summarize the approach you took? Do you think you'll have time to rebase etc? I'd like ideally to get @eddyb to buy in to the overall approach. =)
kennytm commentedon Apr 25, 2018
The prototype implementation works like this (note that
#[track_caller]
was called#[implicit_caller_location]
there):First there will be an AST transformation pass (implemented as an attribute proc-macro in
src/libsyntax_ext/implicit_caller_location.rs
) which expands:into
The purpose of this pass is to conveniently make up a new
DefId
.Create a new MIR inlining pass (main implementation in
src/librustc_mir/transform/inline.rs
). This pass does two things:Force-inline any functions calls
foo()
where the attributes of the calleefoo
contains#[rustc_track_caller]
.If
intrinsics::caller_location()
is called :-__closure
, replace it by the argument__location
. This is to propagate the caller location.Define the
caller_location()
intrinsic.Because
Location
now needs to be known by the compiler, make the Location struct a lang-item.Update the standard library to use track-caller features:
panic!
to usecaller_location()
(or its safe counterpart,Location::caller()
)#[track_caller]
tounwrap()
andexpect()
etc.Implement the
-Z location-detail
flag (search forlocation_detail
) so that the filename/line/column can be redacted.eddyb commentedon Apr 25, 2018
I think a syntactical transformations is unnecessary because we can instead change the "direct call ABI" (vs reifying to a fn pointer, including trait vtables via miri, which could go through a MIR shim).
MIR inlining should also not be needed.
kennytm commentedon Apr 26, 2018
@eddyb So you are suggesting to change the
extern "Rust"
ABI to always pass an extra argument?eddyb commentedon Apr 26, 2018
@kennytm Only for functions declared with the attribute and called through static dispatch, everything else would use a shim when reifying (which I think we do already in some other cases).
kennytm commentedon Apr 26, 2018
@eddyb Maybe you could write down some mentoring instructions i.e. which files to look at 😊
nikomatsakis commentedon May 2, 2018
@eddyb do it! do it! I want this feature.
131 remaining items
nikomatsakis commentedon Jul 29, 2020
Sure, I guess so. Thanks again @anp for seeing this through!
panic!
Backtrace. Code in listing 9-1 is wrong/outdated. book#2550schreter commentedon Mar 31, 2022
Hi. I know this is a bit off-topic and this particular issue is already closed, but I suppose, the right people are involved here :-), so someone might help.
I implemented for our project a version of
Result
which usesTry
trait in such a way as to track the error handling locations, so we get a "call stack" of the error at the end. This works in conjunction with#[track_caller]
very well and we see the locations the error handling took.However, there is one major deficiency in
Location
- it only gives us the source name and line. Yes, with that, it's possible to manually look up the function where it happened, but it would be significantly simpler to evaluate bug reports by looking at the call trace with function names (we have practiced this in a large C++ project, where basically the dev support standardized on initially evaluating everything by function names call trace, ignoring line numbers). So it would come extremely handy if theLocation
could be extended with another&str
containing the function name (ideally with generics). It can be also a mangled name, I don't care much, but the function name is important.Before you should "backtrace!" - yes, but... We are using heavy asynchronous processing handling errors across
await
s, where the backtrace has about zero value. Similar for tracing, we can't just always trace due to performance reasons. So the error handling "call stack" is a perfect compromise - cheap and sufficiently valuable (except that it's missing the function name).Any suggestion where/how to address this issue (
Location
extension by file name)? According to my code study of the Rust compiler code, it should basically boil down to getting the function name from the currentSpan
and adding it in addition to the file name to the generatedconst Location
here:rust/compiler/rustc_codegen_cranelift/src/common.rs
Line 351 in 012720f
Thanks & regards,
Ivan
jplatte commentedon Mar 31, 2022
I remember asking about this quite a while ago and getting a response along the lines of "unlikely to happen for perf or code size reasons". Unfortunately I can't find an issue about it, so I guess it must have been on Zulip.
I think creating a separate issue would be more useful than continuing this conversation here, in any case.
schreter commentedon Mar 31, 2022
Well, it's clear that this will increase generated constants segment by potentially quite a bit, because instead of a single string per file we'll need to store many strings per file. So it could be made optional at compilation time. But I personally don't think it'll be significantly slower (during compilation; runtime is obviously unaffected).
Sure. Simply a new top-level issue w/o any special tags? The question is how to get it to the attention of relevant people who could do something about it.
est31 commentedon Mar 31, 2022
If you have the source code, it should be quite easy to build a tool that translates the Location to the name of the function.
Location
contains the file name, as well as line and column, so all you need to do is go to that specific line and column, and then parse the next ident. Done easily withsyn
where you can write a custom visitor with avisit_ident
function, like this. Then you just compare the line/col number with the line/col number of theLocation
, done. Maybe if the source code is not public you can export theLocation
via e.g. json and import it via the tool.schreter commentedon Mar 31, 2022
Of course it could be translated. But that's another step, which makes it quite cumbersome. The compiler already knows it at compile time and aside from costing more space in the generated executable (string section), there should be no adverse effects of having the function name in the
Location
. It would in general allow building various tools which use the location for debugging purposes, which in turn would help the community in general.Let me move this to a new issue, for further discussion.
Location
is missing the function name #95529