-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Make some invalid codegen attr errors structured/translatable #137279
base: master
Are you sure you want to change the base?
Conversation
r? @nnethercote rustbot has assigned @nnethercote. Use |
Some changes occurred in compiler/rustc_codegen_ssa/src/codegen_attrs.rs |
I thought we were in limbo w.r.t. translatable/structured diagnostics. https://rust-lang.zulipchat.com/#narrow/channel/147480-t-compiler.2Fwg-diagnostics/topic/.E2.9C.94.20translation.20effort I kinda don't want to land more PRs to make things into translatable diagnostics for no reason if it's not clear that this effort has a primary owner in the compiler and it's possible that we may just want to rip it all out? |
(idk what's the plan for the translation infra, I recorded what I found out about the current status over at #132181) AFAICT, the current status is that:
But yeah, probably doesn't really make sense to migrate existing diagnostics over to the translatable one unless it makes the code nicer or if we have a path forward for the translatable infra, I guess. FWIW, ripping the translatable infra out is equally churny/disruptive so... |
I wasn't aware of the policy change discussions, only vaguely remembered a different thread from davidtwco about some of the issues he saw with the current approach. I'm ok with not landing these. But I think there's still value in the current infrastructure even independently of the (demise of the?) translation effort. One case is #137201, where I made it so that instead of having to manually shorten types and set the message for where the full type name was written on every error, the translation machinery handles that on every appropriate type without manual intervention. |
I will say r=me because it looks like conversion is valid, code-wise, and it gets rid of a few calls to |
☔ The latest upstream changes (presumably #137497) made this pull request unmergeable. Please resolve the merge conflicts. |
No description provided.