Suggest comma when missing in macro call #53183

estebank · 2018-08-08T05:34:00Z

When missing a comma in a macro call, suggest it, regardless of
position. When a macro call doesn't match any of the patterns, check
if the call's token stream could be missing a comma between two idents,
and if so, create a new token stream containing the comma and try to
match against the macro patterns. If successful, emit the suggestion.

This works on arbitrary macros, with no need of special support from
the macro writers.

error: no rules expected the token `d`
  --> $DIR/missing-comma.rs:26:18
   |
LL |     foo!(a, b, c d, e);
   |                 -^
   |                 |
   |                 help: missing comma here

Follow up to #52397.

When missing a comma in a macro call, suggest it, regardless of position. When a macro call doesn't match any of the patterns, check if the call's token stream could be missing a comma between two idents, and if so, create a new token stream containing the comma and try to match against the macro patterns. If successful, emit the suggestion.

rust-highfive · 2018-08-08T05:34:03Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

estebank · 2018-08-08T05:34:14Z

cc @oli-obk @dtolnay

estebank · 2018-08-08T05:35:51Z

src/libsyntax/ext/tt/macro_rules.rs

@@ -181,7 +181,7 @@ fn generic_extension<'cx>(cx: &'cx mut ExtCtxt,
        for lhs in lhses { // try each arm's matchers
            let lhs_tt = match *lhs {
                quoted::TokenTree::Delimited(_, ref delim) => &delim.tts[..],
-                _ => cx.span_bug(sp, "malformed macro lhs")
+                _ => continue,


This is in the recovery path, we shouldn't be hitting this ever, as it would be caught above, but if we were we don't really care.

estebank · 2018-08-08T05:36:23Z

src/libsyntax/tokenstream.rs

@@ -186,21 +186,43 @@ impl TokenStream {
    /// Given a `TokenStream` with a `Stream` of only two arguments, return a new `TokenStream`
    /// separating the two arguments with a comma for diagnostic suggestions.
    pub(crate) fn add_comma(&self) -> Option<(TokenStream, Span)> {
-        // Used to suggest if a user writes `println!("{}" a);`
+        // Used to suggest if a user writes `foo!(a b);`


println case is being handled elsewhere, as it is no longer affected by this code since it no longer uses concat.

estebank · 2018-08-08T05:38:31Z

src/test/ui/macros/missing-comma.stderr

+  --> $DIR/missing-comma.rs:28:18
+   |
+LL |     foo!(a, b, c d e);
+   |                  ^


We purposely don't attempt further resolutions as the further we go from correct code the harder it'll be to fix automatically, but people have common cases for typos (missing =>? extra comma?), we could expand this code to handle it on a best effort manner.

oli-obk

@bors r+ rollup

oli-obk · 2018-08-08T07:11:51Z

src/libsyntax/tokenstream.rs

+            let mut suggestion = None;
+            let mut iter = slice.iter().enumerate().peekable();
+            while let Some((pos, ts)) = iter.next() {
+                if let Some((_, next)) = iter.peek() {


Neat trick. This solves most of the issues I guess. If it keeps coming up we can consider a generalized version that doesn't hardcode specific tokens

For the record, my original idea was to try to do this in

rust/src/libsyntax/ext/tt/macro_rules.rs

Line 109 in 3f4f18f

let mut best_fail_tok = None;

and collect all failures with their possible expectations. But I'm not sure what kind of parser changes that would have required.

Let's create a follow up ticket. The current approach was slightly less error prone and easier to implement, but that would be more helpful for more macros.

Mark-Simulacrum · 2018-08-08T12:58:42Z

@bors r=oli-obk rollup

bors · 2018-08-08T12:58:43Z

📌 Commit f4039af has been approved by oli-obk

Suggest comma when missing in macro call When missing a comma in a macro call, suggest it, regardless of position. When a macro call doesn't match any of the patterns, check if the call's token stream could be missing a comma between two idents, and if so, create a new token stream containing the comma and try to match against the macro patterns. If successful, emit the suggestion. This works on arbitrary macros, with no need of special support from the macro writers. ``` error: no rules expected the token `d` --> $DIR/missing-comma.rs:26:18 | LL | foo!(a, b, c d, e); | -^ | | | help: missing comma here ``` Follow up to rust-lang#52397.

Rollup of 15 pull requests Successful merges: - #52773 (Avoid unnecessary pattern matching against Option and Result) - #53082 (Fix doc link (again)) - #53094 (Automatically expand section if url id point to one of its component) - #53106 (atomic ordering docs) - #53110 (Account for --remap-path-prefix in save-analysis) - #53116 (NetBSD: fix signedess of char) - #53179 (Whitelist wasm32 simd128 target feature) - #53183 (Suggest comma when missing in macro call) - #53207 (Add individual docs for rotate_{left, right}) - #53211 ([nll] enable feature(nll) on various crates for bootstrap) - #53214 ([nll] enable feature(nll) on various crates for bootstrap: part 2) - #53215 (Slightly refactor syntax_ext/format) - #53217 (inline some short functions) - #53219 ([nll] enable feature(nll) on various crates for bootstrap: part 3) - #53222 (A few cleanups for rustc_target)

rust-highfive assigned eddyb Aug 8, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 8, 2018

estebank commented Aug 8, 2018

View reviewed changes

oli-obk approved these changes Aug 8, 2018

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 8, 2018

This was referenced Aug 8, 2018

Rollup of 14 pull requests #53194

Closed

Rollup of 14 pull requests #53195

Closed

cramertj mentioned this pull request Aug 8, 2018

Rollup of 13 pull requests #53197

Closed

cramertj mentioned this pull request Aug 8, 2018

Rollup of 12 pull requests #53206

Closed

kennytm mentioned this pull request Aug 9, 2018

Rollup of 15 pull requests #53216

Merged

bors merged commit f4039af into rust-lang:master Aug 9, 2018

pnkfelix mentioned this pull request Oct 8, 2018

[1.30 beta] Test suite of the jemalloc-ctl crate is failing #54478

Closed

estebank deleted the println-comma branch November 9, 2023 05:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggest comma when missing in macro call #53183

Suggest comma when missing in macro call #53183

estebank commented Aug 8, 2018 •

edited

Loading

rust-highfive commented Aug 8, 2018

estebank commented Aug 8, 2018 •

edited

Loading

estebank Aug 8, 2018

estebank Aug 8, 2018

estebank Aug 8, 2018

oli-obk left a comment

oli-obk Aug 8, 2018

oli-obk Aug 8, 2018

estebank Aug 8, 2018

Mark-Simulacrum commented Aug 8, 2018

bors commented Aug 8, 2018

Suggest comma when missing in macro call #53183

Suggest comma when missing in macro call #53183

Conversation

estebank commented Aug 8, 2018 • edited Loading

rust-highfive commented Aug 8, 2018

estebank commented Aug 8, 2018 • edited Loading

estebank Aug 8, 2018

Choose a reason for hiding this comment

estebank Aug 8, 2018

Choose a reason for hiding this comment

estebank Aug 8, 2018

Choose a reason for hiding this comment

oli-obk left a comment

Choose a reason for hiding this comment

oli-obk Aug 8, 2018

Choose a reason for hiding this comment

oli-obk Aug 8, 2018

Choose a reason for hiding this comment

estebank Aug 8, 2018

Choose a reason for hiding this comment

Mark-Simulacrum commented Aug 8, 2018

bors commented Aug 8, 2018

estebank commented Aug 8, 2018 •

edited

Loading

estebank commented Aug 8, 2018 •

edited

Loading