Add on_unmatch_args by chenyukang · Pull Request #154794 · rust-lang/rust

chenyukang · 2026-04-04T02:23:56Z

View all comments

Fixes #152494
r? @estebank

rustbot · 2026-04-04T02:23:58Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

mejrs

Thanks, great idea. I love this.

A few points

you need more tests, for invalid args, on invalid items, cross crate use, format parameters etc.
format parameters; do these make sense? Currently they are not handled at all, just silently ignored. In check_attr.rs you can call directive_visit_params and issue diagnostics for them. Would it be useful to have indexed parameters like {1}, {2} to reference arguments? Something to think about, perhaps.
2c on the naming; I don't have a problem with the name perse, I just intensely dislike names that are really long. Also you don't have to use the exact name estebank suggested :) Something shorter would be lovely. Also consider whether this is something we could extend to functions etc?
Can you please add some docs about this in the unstable book: https://doc.rust-lang.org/nightly/unstable-book/

View changes since this review

chenyukang · 2026-04-04T10:20:01Z

for naming, maybe on_missing_args maybe better, incase we can extend it in future?

mejrs

Thanks for addressing my earlier comments (I cannot click resolve myself 😮‍💨).

Finally, I just remembered that declarative macro derives and attributes exist - can you add tests (and perhaps support for them) as well? See #145208 and #144579.

View changes since this review

…JonathanBrouwer Stabilize check-cfg suggestions for symbol When I was working on rust-lang#154794, I found a weird CI fail rust-lang#154794 (comment), I finally found this caused by unstable cfg suggestions by using `FxHashSet`(from PR rust-lang#154777). Because my PR by luck insert a new symbol, which makes the final diagnostic order changed. It's a standalone issue, so it's better to fix in a separate PR.

Rollup merge of #154886 - chenyukang:yukang-fix-cfg-sugg, r=JonathanBrouwer Stabilize check-cfg suggestions for symbol When I was working on #154794, I found a weird CI fail #154794 (comment), I finally found this caused by unstable cfg suggestions by using `FxHashSet`(from PR #154777). Because my PR by luck insert a new symbol, which makes the final diagnostic order changed. It's a standalone issue, so it's better to fix in a separate PR.

rustbot · 2026-04-19T13:52:37Z

Some changes occurred to diagnostic attributes.

cc @mejrs

chenyukang · 2026-04-19T13:52:46Z

@rustbot ready

mejrs

I'm happy with the implementation, just a couple of nits - see comments.

We also need more tests documenting the current behavior (if any) when applied to macro_rules! derive and attribute macros, as well as all the procedural macro types. It's up to you if you do that in this PR or add it as todos on the tracking issue. I think I prefer the latter though.

@estebank do you have any comments? If not, r=me with nits fixed.

View changes since this review

mejrs · 2026-04-19T17:24:22Z

+    message = "pair! is missing its second argument",
+    label = "add the missing value here",
+    note = "this macro expects a type and a value, like `pair!(u8, 0)`",


The message and label were written with the "on missing args" semantics; they feel a little off now.

My suggestion:

#[diagnostic::on_unmatch_args( message = "invalid arguments to {This} macro invocation", label = "expected a type and value here", note = "this macro expects a type and a value, like `pair!(u8, 0)`", note = "see <link/to/docs>" )]

Thoughts?

The same is true for many tests, but those don't really matter I suppose.

mejrs · 2026-04-19T17:33:00Z

+}
+
+pair!(u8);
+```


Please also show the compiler output below the example.

mejrs · 2026-04-19T18:15:54Z

 /// [`offset_of_slice`]: https://doc.rust-lang.org/nightly/unstable-book/language-features/offset-of-slice.html
 #[stable(feature = "offset_of", since = "1.77.0")]
+#[diagnostic::on_unmatch_args(
+    note = "this macro expects a container type and a dot-separated field or variant path, like `offset_of!(Type, field)`"


Suggested change

note = "this macro expects a container type and a dot-separated field or variant path, like `offset_of!(Type, field)`"

note = "this macro expects a container type and a (nested) field path, like `offset_of!(Type, field)`"

offset_of! for enums is unstable, we shouldn't suggest it.

mejrs · 2026-04-22T10:32:37Z

r? me

Sorry, one more thing I just noticed. In rustc_feature and the unstable book you reference #152494 as the tracking issue, but that is estebank's suggestion issue, not a tracking issue. Please create and link a proper tracking issue.

r=me with tracking issue

chenyukang · 2026-04-22T11:13:44Z

r? me

Sorry, one more thing I just noticed. In rustc_feature and the unstable book you reference #152494 as the tracking issue, but that is estebank's suggestion issue, not a tracking issue. Please create and link a proper tracking issue.

r=me with tracking issue

ok, I placed it as a placeholder link and forget it 😅
I will create a new tracking issue.

rustbot · 2026-04-22T11:29:16Z

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.

mejrs · 2026-04-22T12:08:04Z

Thanks for working on this :)

@bors r+

rust-bors · 2026-04-22T12:08:07Z

📌 Commit 8d75f0c has been approved by mejrs

It is now in the queue for this repository.

…uwer Rollup of 10 pull requests Successful merges: - #154794 (Add on_unmatch_args) - #155133 (Document precision considerations of `Duration`-float methods) - #154283 (Remove `nodes_in_current_session` field and related assertions) - #155374 (rustdoc: fix a few spots where emit isn't respected) - #155587 (Immediately feed visibility on DefId creation) - #155622 (c-variadic: `va_arg` fixes ) - #155629 (rustc_public: Add `constness` & `asyncness` in `FnDef`) - #155632 (Some metadata cleanups) - #155639 (BinOpAssign always returns unit) - #155647 (rustc-dev-guide subtree update)

@estebank

Rollup merge of #154794 - chenyukang:yukang-fix-152494-incomplete-macro-args, r=mejrs Add on_unmatch_args Fixes #152494 r? @estebank

…uwer Rollup of 10 pull requests Successful merges: - rust-lang/rust#154794 (Add on_unmatch_args) - rust-lang/rust#155133 (Document precision considerations of `Duration`-float methods) - rust-lang/rust#154283 (Remove `nodes_in_current_session` field and related assertions) - rust-lang/rust#155374 (rustdoc: fix a few spots where emit isn't respected) - rust-lang/rust#155587 (Immediately feed visibility on DefId creation) - rust-lang/rust#155622 (c-variadic: `va_arg` fixes ) - rust-lang/rust#155629 (rustc_public: Add `constness` & `asyncness` in `FnDef`) - rust-lang/rust#155632 (Some metadata cleanups) - rust-lang/rust#155639 (BinOpAssign always returns unit) - rust-lang/rust#155647 (rustc-dev-guide subtree update)

rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label Apr 4, 2026

rustbot assigned estebank Apr 4, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 4, 2026

chenyukang force-pushed the yukang-fix-152494-incomplete-macro-args branch from 3eb0c39 to e3e5e61 Compare April 4, 2026 02:29

mejrs reviewed Apr 4, 2026

View reviewed changes

Comment thread compiler/rustc_attr_parsing/src/attributes/diagnostic/on_unmatch_args.rs

Comment thread compiler/rustc_expand/src/mbe/macro_rules.rs

Comment thread compiler/rustc_expand/src/mbe/diagnostics.rs Outdated

chenyukang force-pushed the yukang-fix-152494-incomplete-macro-args branch from e3e5e61 to 75df17e Compare April 6, 2026 10:48

chenyukang changed the title ~~Add on_incomplete_macro_args~~ Add on_missing_args Apr 6, 2026