-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Implement -Z location-detail flag #89920
Conversation
Some changes occured to the CTFE / Miri engine cc @rust-lang/miri |
r? @davidtwco (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
2d5b22d
to
017d4d9
Compare
This comment has been minimized.
This comment has been minimized.
017d4d9
to
0f91d0d
Compare
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.
LGTM, r=me once these comments are resolved.
0f91d0d
to
b802629
Compare
I added a |
@bors r+ |
📌 Commit b802629 has been approved by |
…l, r=davidtwco Implement -Z location-detail flag This PR implements the `-Z location-detail` flag as described in rust-lang/rfcs#2091 . `-Z location-detail=val` controls what location details are tracked when using `caller_location`. This allows users to control what location details are printed as part of panic messages, by allowing them to exclude any combination of filenames, line numbers, and column numbers. This option is intended to provide users with a way to mitigate the size impact of `#[track_caller]`. Some measurements of the savings of this approach on an embedded binary can be found here: rust-lang#70579 (comment) . Closes rust-lang#70580 (unless people want to leave that open as a place for discussion of further improvements). This is my first real PR to rust, so any help correcting mistakes / understanding side effects / improving my tests is appreciated :) I have one question: RFC 2091 specified this as a debugging option (I think that is what -Z implies?). Does that mean this can never be stabilized without a separate MCP? If so, do I need to submit an MCP now, or is the initial RFC specifying this option sufficient for this to be merged as is, and then an MCP would be needed for eventual stabilization?
…l, r=davidtwco Implement -Z location-detail flag This PR implements the `-Z location-detail` flag as described in rust-lang/rfcs#2091 . `-Z location-detail=val` controls what location details are tracked when using `caller_location`. This allows users to control what location details are printed as part of panic messages, by allowing them to exclude any combination of filenames, line numbers, and column numbers. This option is intended to provide users with a way to mitigate the size impact of `#[track_caller]`. Some measurements of the savings of this approach on an embedded binary can be found here: rust-lang#70579 (comment) . Closes rust-lang#70580 (unless people want to leave that open as a place for discussion of further improvements). This is my first real PR to rust, so any help correcting mistakes / understanding side effects / improving my tests is appreciated :) I have one question: RFC 2091 specified this as a debugging option (I think that is what -Z implies?). Does that mean this can never be stabilized without a separate MCP? If so, do I need to submit an MCP now, or is the initial RFC specifying this option sufficient for this to be merged as is, and then an MCP would be needed for eventual stabilization?
…askrgr Rollup of 11 pull requests Successful merges: - rust-lang#83233 (Implement split_array and split_array_mut) - rust-lang#88300 (Stabilise unix_process_wait_more, extra ExitStatusExt methods) - rust-lang#89416 (nice_region_error: Include lifetime placeholders in error output) - rust-lang#89468 (Report fatal lexer errors in `--cfg` command line arguments) - rust-lang#89730 (add feature flag for `type_changing_struct_update`) - rust-lang#89920 (Implement -Z location-detail flag) - rust-lang#90070 (Add edition configuration to compiletest) - rust-lang#90087 (Sync rustfmt subtree) - rust-lang#90117 (Make RSplit<T, P>: Clone not require T: Clone) - rust-lang#90122 (CI: make docker cache download and `docker load` time out after 10 minutes) - rust-lang#90166 (Add comment documenting why we can't use a simpler solution) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
How do you use this flag to disable ALL the location details? When I set
|
I think I misunderstood how CLI option parsing works when implementing this, I thought passing an empty string to the option would pass |
#99620 contains the fix |
…=davidtwco `-Z location-detail`: provide option to disable all location details As reported [here](rust-lang#89920 (comment)), when I first implemented the `-Z location-detail` flag there was a bug, where passing an empty list was not correctly supported, and instead rejected by the compiler. This PR fixes that such that passing an empty list results in no location details being tracked, as originally specified in rust-lang/rfcs#2091 . This PR also adds a test case to verify that this option continues to work as intended.
This PR implements the
-Z location-detail
flag as described in rust-lang/rfcs#2091 .-Z location-detail=val
controls what location details are tracked when usingcaller_location
. This allows users to control what location details are printed as part of panic messages, by allowing them to exclude any combination of filenames, line numbers, and column numbers. This option is intended to provide users with a way to mitigate the size impact of#[track_caller]
.Some measurements of the savings of this approach on an embedded binary can be found here: #70579 (comment) .
Closes #70580 (unless people want to leave that open as a place for discussion of further improvements).
This is my first real PR to rust, so any help correcting mistakes / understanding side effects / improving my tests is appreciated :)
I have one question: RFC 2091 specified this as a debugging option (I think that is what -Z implies?). Does that mean this can never be stabilized without a separate MCP? If so, do I need to submit an MCP now, or is the initial RFC specifying this option sufficient for this to be merged as is, and then an MCP would be needed for eventual stabilization?