Introduce #[diagnostic::on_move(message)]#150935
Introduce #[diagnostic::on_move(message)]#150935rperier wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
|
rustbot has assigned @JonathanBrouwer. Use |
|
r? @estebank |
|
I'm currently working on migrating the existing diagnostic attribute infrastructure, can you do this PR after that? It's going to be quite a mess to resolve that conflict. Is this meant to be only used by the standard library or also by users? If only by std, I think you should forego the attribute and instead check whether T implements |
|
Yeah I can completly wait until you finished your work and do my PR after that, It was my intention to be honest . Resolving conflicts or rebasing is not an issue for me. |
9a0162d to
8071a7d
Compare
|
Some changes occurred in compiler/rustc_attr_parsing cc @jdonszelmann, @JonathanBrouwer Changes to the size of AST and/or HIR nodes. cc @nnethercote Some changes occurred in compiler/rustc_hir/src/attrs |
This comment has been minimized.
This comment has been minimized.
|
I have reworked my code and moved everything to
|
8071a7d to
09506a4
Compare
This comment has been minimized.
This comment has been minimized.
09506a4 to
829651a
Compare
This comment has been minimized.
This comment has been minimized.
|
Rebased onto main, I have also fixed the size of |
| #[diagnostic::on_move( | ||
| message = Foo, | ||
| //~^ERROR expected a literal (`1u8`, `1.0f32`, `"string"`, etc.) here, found expression | ||
| label = "Bar", | ||
| )] | ||
| #[diagnostic::on_move( | ||
| message = "Foo" | ||
| label = "Bar", | ||
| //~^ERROR expected `,`, found `label` | ||
| )] |
There was a problem hiding this comment.
This can't be an error; this must be a lint. But you will need #151516 to do so.
There was a problem hiding this comment.
Hi. Since this morning, these "cases" are no longer handled at all. The parser does not report anything and I get an empty MetaItemListParser (there are no sub_parsers) in onMoveParser::convert , see #t-compiler/help > #diagnostic::on_move(message) (PR #150935) @ 💬 for the full description. Is it expected ? I mean I cannot emit the lint myself, I have an empty ArgParser context, and the parser reports nothing.
There was a problem hiding this comment.
I think, I got it. This is due to https://github.com/rust-lang/rust/blob/main/compiler/rustc_attr_parsing/src/parser.rs#L132 . I can of course emit a lint myself when the MetaItemListParser is empty, it's a shame to no longer have the context of the error, we had a more fine grained context with a span before. The span now will be the full diagnostic attribute.
There was a problem hiding this comment.
It is possible to replace the entire span https://github.com/rust-lang/rust/blob/main/compiler/rustc_attr_parsing/src/parser.rs#L135 by e.span (or build a span from e) ? so we would have a span on the attr in this case instead of the whole diagnostic attribute.
| #[derive(Clone, Debug, Encodable, Decodable, HashStable_Generic, PrintAttribute)] | ||
| pub struct OnMoveAttrArg { | ||
| pub symbol: Symbol, | ||
| pub format_ranges: ThinVec<(usize, usize)>, | ||
| } |
There was a problem hiding this comment.
I plan on moving a fair bit of the format and string stuff that on_unimplemented uses into rustc_hir; you should be able to reuse some of that in this PR :)
829651a to
b12aa8b
Compare
This comment has been minimized.
This comment has been minimized.
|
Rebased onto main. I have covered changes related to #151516 .
If I can help for this (by proposing a patch for a generic lint, for example), feel free to ask. |
This comment has been minimized.
This comment has been minimized.
b12aa8b to
1efb939
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This might be helpful for smart pointers to explains why they aren't Copy and what to do instead or just to let the user know that .clone() is very cheap and can be called without a performance penalty.
1efb939 to
7d94896
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Fixed merge conflicts and rebased onto main |
|
☔ The latest upstream changes (presumably #152104) made this pull request unmergeable. Please resolve the merge conflicts. |
|
There has not been an RFC for this attribute, please create an RFC to do so. Per the diagnostics rfc:
|
cc #149862
This is a first proposal. I have deliberately kept it simpler than
diagnostic::on_unimplemented.Few questions/remarks:
rustc_astfrom the borrowck ?Suggestions are welcomed !