Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Rust] Macro improperly marked as invalid #3904

Open
Rapptz opened this issue Jan 14, 2024 · 3 comments · May be fixed by #3912
Open

[Rust] Macro improperly marked as invalid #3904

Rapptz opened this issue Jan 14, 2024 · 3 comments · May be fixed by #3912
Labels
C: Syntax T: bug A bug in an existing language feature

Comments

@Rapptz
Copy link

Rapptz commented Jan 14, 2024

What happened?

The following macro gives an erroneous source.rust meta.macro.rust invalid.illegal.rust scope:

macro_rules! test {
    ($($name:ident),+$(,)?) => {
        pub struct Foo {
            $(
                pub $name : Option<String>
            ),+
        }

        pub async fn thing() -> () {}
    }
}

image

From a cursory view in the repository and from testing it seems this is applied because of this rule

macro-semi-sep:
- include: comments
- match: ';'
scope: punctuation.terminator.rust
pop: true
- match: '(?=[})\]])'
pop: true
- match: '\S'
# This is intended to help make it evident when you forget a semicolon.
scope: invalid.illegal.rust

Adding the ; after the struct definition does make the scope go away but ironically it leads to invalid code since you can't have ; after a struct definition:

error: expected item, found `;`
  --> src/main.rs:15:2
   |
15 | };
   |  ^ help: remove this semicolon
   |
   = help: braced struct declarations are not followed by a semicolon
@deathaxe deathaxe added T: bug A bug in an existing language feature C: Syntax labels Jan 14, 2024
@FichteFoll FichteFoll linked a pull request Jan 28, 2024 that will close this issue
@FichteFoll
Copy link
Collaborator

FichteFoll commented Jan 28, 2024

The issue was a bit more involved than you initially suspected because the problem was that macro repetitions inside transcribers where basically unsupported. The linked PR #3912 fixes this particular issue for structs, but due to a lack of real world examples for other block statements I didn't check if they exhibit the same or similar issues.

If you have some more complex examples for macros, that would be much appreciated.

Edit: Well, didn't take long for me to find a problem with that implementation. No promises when I'll find time to work on this again, considering that I also don't have an idea how to resolve it currently

@Rapptz
Copy link
Author

Rapptz commented Jan 30, 2024

At first glance, I couldn't find any other macros that error out (that use repetitions) outside of this (partially censored) one:

#[macro_export]
macro_rules! metadata {
    (
        namespace $ns:literal;
        $(
            $name:ident: $ity:ty
        ),+$(,)?
    ) => {
        #[derive(Debug, Clone, Default, Eq, PartialEq)]
        pub struct Metadata {
            $(
                $name: Option<$ity>
            ),+
        }

        impl $crate::database::Metadata for Metadata {
            const NAMESPACE: &'static str = $ns;
        }
    }
}

The others that I use that have repetitions currently do not error, e.g.

#[macro_export]
macro_rules! header_map {
    ($($name:literal => $value:expr),* $(,)*) => {{
        let mut headers = reqwest::header::HeaderMap::with_capacity($crate::__count!($($name)*));
        $(
            headers.insert(
                reqwest::header::HeaderName::from_static($name),
                reqwest::header::HeaderValue::from_static($value),
            );
        )*

        headers
    }};
}

@FichteFoll
Copy link
Collaborator

Yeah, it's (multiple) macro transcribers inside other blocks such as a struct definition that are a problem here (or similarly for enums). Since they can appear just about everywhere, I'd prefer a generic solution on the syntax definition side over a specific one, but it may just not be that easy to realize because of state tracking … Will need to ponder about it a bit.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C: Syntax T: bug A bug in an existing language feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants