Skip to content

format_args! "decompiler": forced warnings and fmt::Arguments::new_v1_formatted "support". #326

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

Merged
merged 3 commits into from
Jul 7, 2025

Conversation

eddyb
Copy link
Collaborator

@eddyb eddyb commented Jul 7, 2025

(Each commit should be reviewed separately)


The main "externally" visible change is the "force warnings" one - thought it's still mostly just for us.

Due to Cargo using --cap-lints, you normally can't see warnings in non-local dependencies - and that includes the standard library when using -Zbuild-std (like we do).
So every time the "format_args! decompiler" would fail (e.g. while working on a rustup, before updating it) for a panic! in e.g. core, it would silently cause issues downstream (while making it harder to diagnose)

After this PR, it will always warn in the crate responsible (though hopefully never, for Rust-GPU users).


I've had this "force warnings" change around for a while, but never submitted it because there were always a few warnings from core - turns out they were caused by fmt::Arguments::new_v1_formatted, which is used for format_args! with any non-default options (e.g. padding) or non-trivial argument numbering, etc.

I decided to "support" those cases - however, actually extracting the necessary information would be too much effort (and require relying on unstable implementation details of core::fmt::rt), so the debugPrintf form includes all the arguments but in a very general "these N args go into M placeholders somehow".

Also, there's some really hacky skipping of core::fmt::rt::Placeholder::new calls, due to main predating:

Thankfully, that PR landed between 1.87 and 1.88, so #249 (the ~1.88 rustup) should be able to remove the hacks (as the entire &[core::fmt::rt::Placeholder] slice should be a constant then, just like the &[&str] one is).

(In theory, due to the level of simplification done upstream, it may be plausible to start decoding the Placeholder values, but I would not want to put effort into it until an usecase is revealed)


There are also some changes that in theory aren't needed before #320 (the ~1.89 rustup), but which I've found invaluable while debugging any changes to the format_args! "decompiler" logic (and which I wish I had done much sooner).

@eddyb eddyb enabled auto-merge July 7, 2025 10:34
@Firestar99
Copy link
Member

One request, doesn't have to be now if it breaks too much of your future commits, but eventually: Could we move the ~500 line format_args! decompiler monster into a separate function to improve readability of call(), like we already do for the other special cases? Potentially even move it to a separate mod, considering how extensive this feature is?

@eddyb eddyb added this pull request to the merge queue Jul 7, 2025
Copy link
Member

@Firestar99 Firestar99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't fully verify everything, but looks good to me

Also works on my project

@eddyb
Copy link
Collaborator Author

eddyb commented Jul 7, 2025

One request, doesn't have to be now if it breaks too much of your future commits, but eventually: Could we move the ~500 line format_args! decompiler monster into a separate function to improve readability of call(), like we already do for the other special cases?

I'll be honest, I don't remember why it wasn't in a separate method. Frankly, I am tired of updating it, so I am mostly trying to keep it in working condition for the next Rust-GPU release, after which it may become feasible to rewrite it as a much nicer SPIR-T pass (at least after the point where function pointers are representable in SPIR-V, even before recursion/indirect-call emulation), which could also take full advantage of "semantic", instead of "syntactic", pattern-matching (e.g. we could re-enable MIR inlining, and probably get a nice speedup in compile times, plus much less messy IR to deal with etc. - because the SPIR-T pass would run after anything like inlining or replacing variables with their contents, and would act more like partial evaluation).

Merged via the queue into Rust-GPU:main with commit de24a89 Jul 7, 2025
13 checks passed
@eddyb eddyb deleted the fmt-robustness branch July 7, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants