Repository navigation
builder/format_args_decompiler: post-rustup(s) cleanups. - #683
Merged
Merged
Conversation
eddyb
added this pull request to stack #685
October 15, 2026 23:08
Firestar99
approved these changes
Oct 16, 2026
Firestar99
left a comment
Member
There was a problem hiding this comment.
I'm willing to just approve this without really reviewing it deeply, since I've never looked into our fmt_args_decompiler and I feel like I could spent my time more wisely elsewhere right now.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Most of the issues addressed here were introduced in #545:
(reviewing this module was left to me but I hadn't gotten around to looking closer at it since then)
core::fmt::Arguments::new_v1*is gone but dead code handling it was kept aroundcore::panicking::panic_*functions were intercepted, despite only 4 being neededpanic_nounwind_fmt)format_args_decompiler, which we both don't need to, nor want to (e.g. the full list of wrappers could easily change)#[track_caller]-like behavior), and this PR undoes that, but if we really wanted to, we could more closely mimic#[track_caller]through changes to inlining debuginfo and the actual panic-as-debugPrintftransformationtcx.def_path_str(_)broke down insidecoreitself (there's a reasondeclaredemangles the mangled name, that one is by necessity absolute), which was being papered over using spuriousif ... { return Ok(...); }bypasses to silence warnings (without making such panicking callsites work properly)Lastly, while not strictly necessary, I went ahead and tried minimizing the diff from 2964d67, for
crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs, as much as I could, to ensure I understand everything that changed ("pieces" -> "template" parsing), and what didn't (fmt::rt::Argument::new_*calls).(this also enabled fixing conflicts in one of my "future Rust-GPU branches" commits, that I couldn't figure out when I first ran into them)
Should probably be reviewed piece-wise, but the last commit moves a lot of stuff around anyway.