Skip to content

Format macro def with repeat #2388

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

Closed
nrc opened this issue Jan 23, 2018 · 4 comments · Fixed by #4246
Closed

Format macro def with repeat #2388

nrc opened this issue Jan 23, 2018 · 4 comments · Fixed by #4246

Comments

@nrc
Copy link
Member

nrc commented Jan 23, 2018

thread 'main' panicked at 'Box<Any>', /Users/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/rustc-ap-syntax-12.0.0/parse/mod.rs:228:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: std::panicking::rust_panic_with_hook
   5: std::panicking::begin_panic
   6: syntax::parse::filemap_to_stream
   7: syntax::parse::filemap_to_parser
   8: syntax::parse::new_parser_from_source_str
   9: rustfmt_nightly::parse_input
  10: rustfmt_nightly::format_snippet
  11: rustfmt_nightly::macros::rewrite_macro_def
  12: rustfmt_nightly::visitor::FmtVisitor::visit_item
  13: rustfmt_nightly::visitor::FmtVisitor::walk_items
  14: rustfmt_nightly::visitor::FmtVisitor::walk_mod_items
  15: rustfmt_nightly::visitor::FmtVisitor::format_separate_mod
  16: rustfmt_nightly::run
  17: rustfmt::execute
  18: rustfmt::main
  19: std::rt::lang_start::{{closure}}
  20: std::panicking::try::do_call
  21: __rust_maybe_catch_panic
  22: std::rt::lang_start_internal
  23: main

Just running cargo fmt at the root of nrc/graphql. Also happens at nrc/graphql/graphql. I have no idea why.

@nrc nrc added the bug Panic, non-idempotency, invalid code, etc. label Jan 23, 2018
@topecongiro
Copy link
Contributor

A minimal example:

macro lex_err($kind: ident $(, $body: expr)*) {
    Err(QlError::LexError(LexError::$kind($($body,)*)))
}

from macro def in nrc/graphql/src/parser/lexer.rs.

@topecongiro
Copy link
Contributor

The minimal example:

macro foo() {
    $(x)*
}

@topecongiro
Copy link
Contributor

This is due to replace_names() in macros.rs:

https://github.com/rust-lang-nursery/rustfmt/blob/8e6ee4a7625d1c6ccc2021e17c8589718a1f3474/src/macros.rs#L412-L414

We ignore an opening parenthesis after $.

Removing if c.is_alphanumeric() fixes this.

@topecongiro
Copy link
Contributor

Or, rather than removing c.is_alphanumeric(), we should probably allow '(' and ')' to be included in variable name?

@topecongiro topecongiro added poor-formatting a-macros and removed bug Panic, non-idempotency, invalid code, etc. labels Feb 4, 2018
@topecongiro topecongiro changed the title panic in libsyntax on graphql crate Format macro def with repeat Feb 4, 2018
@nrc nrc added the p-low label Jun 24, 2018
ayazhafiz added a commit to ayazhafiz/rustfmt that referenced this issue Jun 8, 2020
Adds regression tests for the following issues which seem to be fixed on
master:

Closes rust-lang#1762
Closes rust-lang#2201
Closes rust-lang#2388
Closes rust-lang#2672
Closes rust-lang#2755
Closes rust-lang#2947
Closes rust-lang#2978
Closes rust-lang#3148
Closes rust-lang#3206

@topecongiro @calebcartwright appologies for the large number of issues
in this commit; if you prefer I can split it up into 2+.
ayazhafiz added a commit to ayazhafiz/rustfmt that referenced this issue Jun 9, 2020
Adds regression tests for the following issues which seem to be fixed on
master:

Closes rust-lang#1762
Closes rust-lang#2388
Closes rust-lang#2672
Closes rust-lang#2755
Closes rust-lang#2947
Closes rust-lang#2978
Closes rust-lang#3148
Closes rust-lang#3206

@topecongiro @calebcartwright appologies for the large number of issues
in this commit; if you prefer I can split it up into 2+.
calebcartwright pushed a commit that referenced this issue Jun 9, 2020
* Prune stale issues

Adds regression tests for the following issues which seem to be fixed on
master:

Closes #1762
Closes #2388
Closes #2672
Closes #2755
Closes #2947
Closes #2978
Closes #3148
Closes #3206

@topecongiro @calebcartwright appologies for the large number of issues
in this commit; if you prefer I can split it up into 2+.

* fixup! Prune stale issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants