diagnostics: hide expansion of builtin-like macros #141314
Conversation
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
compiler/rustc_errors/src/emitter.rs
Outdated
| || aiu.contains(&sym::print_internals) | ||
| || aiu.contains(&sym::liballoc_internals) | ||
| }), | ||
| )), |
There was a problem hiding this comment.
Might be worth pulling everything after the first || out into a separate variable.
Also, I don't really understand how this works, i.e. how allow_internal_unstable is involved. A brief comment explaining it would be useful.
|
Generally looks good, I will r+ once the tests all pass the the suggestion above is addressed. Thanks. @rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
| @@ -193,16 +193,12 @@ error: asm template must be a string literal | |||
| | | |||
| LL | asm!(format!("{{{}}}", 0), in(reg) foo); | |||
| | ^^^^^^^^^^^^^^^^^^^^ | |||
| | | |||
| = note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info) | |||
There was a problem hiding this comment.
Does -Zmacro-backtrace still work for builtin-like macros with this PR?
There was a problem hiding this comment.
Yes, I've added two tests for it.
|
I just recently rejected this change in #138379. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I was unaware of this PR, thanks for linking it. Personally I do not find the This PR targets macros that are mostly just thin wrappers around some unstable/internal function/macro/intrinsic, and if an error happens it's never because the macro itself did something weird. Most of them have specialized diagnostic paths in the compiler anyway. Putting aside the question of whether we should recommend it less, I'm also happy to discuss the metric by which to decide that. Filtering on Finally, consider that there is a cost to showing messages to the user as well. The more text we shove into an user's face, the less attention that user will pay to a particular note or help message that might actually be well targeted and useful. I'm happy to hear yours and others' thoughts about this. |
|
The Miri subtree was changed cc @rust-lang/miri |
|
I think this PR is good. I agree that the removed parts of the messages are low-value.
@petrochenkov, can you explain more? I read that PR and I guess this comment is the most relevant one, but it's hard to tell exactly how the original version of that PR overlaps with this PR. A fresh explanation would be helpful. |
|
I can only rehash what I said in #138379 (comment). |
|
This feels similar to track_caller hiding functions from the panic location and even const eval backtraces. As suggested by some folk, maybe just an annotation per macro would be best |
As of late I've gotten more and more annoyed by notes like
note: this error originates in the macro $crate::__export::format_args.This PR hides that message if it comes from an expansion of a macro that uses features like
fmt_internals. If its expansion is an implementation detail we shouldn't be mentioning it. This covers macros likevec!,format!and so on, but not the more mundane ones likeassert_eq!andtodo!!