Skip to content
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

$( #[$attr:meta] )* $var:ident → error: local ambiguity: multiple parsing options #24189

Closed
andersk opened this issue Apr 8, 2015 · 7 comments · Fixed by #42913
Closed
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)

Comments

@andersk
Copy link
Contributor

andersk commented Apr 8, 2015

I’m writing a macro that parses enum declarations to work around #16920, and I’ve discovered that it can’t handle attributes on the enum variants like I thought it would. Specifically, the matcher $( #[$attr:meta] )* $var:ident fails to match #[doc = "Bar"] Baz, complaining “error: local ambiguity: multiple parsing options: built-in NTs ident ('var') or 1 other options.”

This doesn’t seem to make sense, since an ident cannot start with #.

Small test case:

#![crate_type = "lib"]

macro_rules! foo {
    (
        pub enum $name:ident {
            $( #[$attr:meta] )* $var:ident
        }
    ) => {
        pub enum $name {
            $( #[$attr] )* $var
        }
    };
}

foo! {
    pub enum Foo {
        #[doc = "Bar"] Baz
    }
}
$ rustc --version --verbose
rustc 1.0.0-beta (9854143cb 2015-04-02) (built 2015-04-02)
binary: rustc
commit-hash: 9854143cba679834bc4ef932858cd5303f015a0e
commit-date: 2015-04-02
build-date: 2015-04-02
host: x86_64-unknown-linux-gnu
release: 1.0.0-beta
$ rustc r.rs
r.rs:17:9: 17:10 error: local ambiguity: multiple parsing options: built-in NTs ident ('var') or 1 other options.
r.rs:17         #[doc = "Bar"] Baz
                ^
@alexcrichton alexcrichton added the A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) label Apr 8, 2015
@orthecreedence
Copy link

orthecreedence commented Jul 10, 2016

@andersk Did you ever find a workaround for this? I'm trying to write a macro that parses structs and I'm running into this:

macro_rules! serializable {
    (
        $(#[$struct_meta:meta])*
        pub struct $name:ident {
            $(
                $(#[$field_meta:meta])*
                $field:ident: $type_:ty
            ),* ,
        }
    ) => {
        $(#[$struct_meta])*
        pub struct $name {
            $(
                $(#[$field_meta])*
               $field: $type_
            ),* ,
        }
    }
}

serializable! {
    #[allow(dead_code)]
    /// This is a test
    pub struct Tester {
        #[allow(dead_code)]
        name: String,
    }
}

Yields:

src\util\serialization.rs:180:13: 180:14 error: local ambiguity: multiple parsing options: built-in NTs ident ('field') or 1 other option.
src\util\serialization.rs:180             #[allow(dead_code)]
                                          ^

rustc 1.10.0 (cfcb716cf 2016-07-03)

@orthecreedence
Copy link

orthecreedence commented Jul 10, 2016

Actually, this issue looks like the exact same as #24827

@azriel91
Copy link
Contributor

I managed to implement a workaround while writing a builder macro, which is to have two parts:

  • keep matching on the front token as a meta token, save that into a separate place, and recursively call yourself, passing the rest of the tokens
  • have another rule that matches on the final pattern that no longer has meta attributes

See code at test.rs (sorry for ugliness, this was a proof of concept which evolved a lot).

I'll convert that to a proper crate and document it this weekend.

Example output for FiveBuilder => Five

    #[doc =
          r" hello everyone"]
    struct Five {
        #[doc = r" doc for i32"]
        something: i32,
    }
    #[doc = "Generated struct builder"]
    struct FiveBuilder {
        #[doc = r" doc for i32"]
        something: Option<i32>,
    }
    impl FiveBuilder {
        /// Construct the builder
        pub fn new() -> FiveBuilder { FiveBuilder{something: Some(0),} }
        /// Build the struct
        pub fn build(self) -> Five {
            let something = self.something.unwrap();
            Five{something: something,}
        }
        #[allow(dead_code)]
        /// Specify a value for the $F_NAME field
        fn something(mut self, value: i32) -> Self {
            self.something = Some(value);
            self
        }
    }

@andersk
Copy link
Contributor Author

andersk commented Jan 9, 2017

@orthecreedence: #24827 has idents conflicting with other idents; those conflicts are “legitimate”, given Rust’s left-to-right parsing strategy. In contrast, this is about idents conflicting with tokens that cannot possibly be idents. Simpler example, showing that rustc can’t determine that (e.g.) > is not an ident:

macro_rules! foo {
    ( $( > )* $x:ident ) => { };
}
foo!( > a );
error: local ambiguity: multiple parsing options: built-in NTs ident ('x') or 1 other option.
 --> foo.rs:4:7
  |
4 | foo!( > a );
  |       ^

It seems like the same thing happens with any token in place of >.

@beamspease
Copy link

I'm running into an error in enum_primitive that according to the bug report is blocked on this issue

kennytm added a commit to kennytm/rust that referenced this issue Jul 7, 2017
bors added a commit that referenced this issue Jul 11, 2017
…seyfried

Only match a fragment specifier the if it starts with certain tokens.

When trying to match a fragment specifier, we first predict whether the current token can be matched at all. If it cannot be matched, don't bother to push the Earley item to `bb_eis`. This can fix a lot of issues which otherwise requires full backtracking (#42838).

In this PR the prediction treatment is not done for `:item`, `:stmt` and `:tt`, but it could be expanded in the future.

Fixes #24189.
Fixes #26444.
Fixes #27832.
Fixes #34030.
Fixes #35650.
Fixes #39964.
Fixes the 4th comment in #40569.
Fixes the issue blocking #40984.
@Yamakaky
Copy link
Contributor

Yamakaky commented Jul 11, 2017

So this issue is solved? (at least on nightly)

@andersk
Copy link
Contributor Author

andersk commented Jul 11, 2017

@Yamakaky It’s marked as closed, isn’t it? But you’ll have to wait for the 2017-07-11 nightly, which hasn’t been built yet.

bors bot added a commit to nix-rust/nix that referenced this issue Jul 23, 2017
661: Allow doc attributes in ioctl r=asomers

fixes #571 . Note that this is a breaking change because it also changes 

```
ioctl!(some_name with 12);
```

to

```
ioctl!(bad some_name with 12);
```

This is to work around a bug in the rust compiler whereby rules around matching idents are overly strict. See rust-lang/rust#24189

It doesn't break anything else though.
bors bot added a commit to open-i18n/rust-unic that referenced this issue Aug 10, 2017
48: Char property macro 2.0 r=behnam

Replaces #41. See #41 for earlier discussion.

An example will show better than I can tell:

```rust
char_property! {
    /// Represents the Unicode character
    /// [*Bidi_Class*](http://www.unicode.org/reports/tr44/#Bidi_Class) property,
    /// also known as the *bidirectional character type*.
    ///
    /// * <http://www.unicode.org/reports/tr9/#Bidirectional_Character_Types>
    /// * <http://www.unicode.org/reports/tr44/#Bidi_Class_Values>
    pub enum BidiClass {
        /// Any strong left-to-right character
        ///
        /// ***General Scope***
        ///
        /// LRM, most alphabetic, syllabic, Han ideographs,
        /// non-European or non-Arabic digits, ...
        LeftToRight {
            abbr => L,
            long => Left_To_Right,
            display => "Left-to-Right",
        }

        /// Any strong right-to-left (non-Arabic-type) character
        ///
        /// ***General Scope***
        ///
        /// RLM, Hebrew alphabet, and related punctuation
        RightToLeft {
            abbr => R,
            long => Right_To_Left,
            display => "Right-to-Left",
        }

        /// Any strong right-to-left (Arabic-type) character
        ///
        /// ***General Scope***
        ///
        /// ALM, Arabic, Thaana, and Syriac alphabets,
        /// most punctuation specific to those scripts, ...
        ArabicLetter {
            abbr => AL,
            long => Arabic_Letter,
            display => "Right-to-Left Arabic",
        }
    }
}

/// Abbreviated name bindings for the `BidiClass` property
pub mod abbr_names for abbr;
/// Name bindings for the `BidiClass` property as they appear in Unicode documentation
pub mod long_names for long;
```

expands to:

```rust
/// Represents the Unicode character
/// [*Bidi_Class*](http://www.unicode.org/reports/tr44/#Bidi_Class) property,
/// also known as the *bidirectional character type*.
///
/// * <http://www.unicode.org/reports/tr9/#Bidirectional_Character_Types>
/// * <http://www.unicode.org/reports/tr44/#Bidi_Class_Values>
#[allow(bad_style)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
pub enum BidiClass {
    /// Any strong left-to-right character
    LeftToRight,
    /// Any strong right-to-left (non-Arabic-type) character
    RightToLeft,
    /// Any strong right-to-left (Arabic-type) character
    ArabicLetter,
}
/// Abbreviated name bindings for the `BidiClass` property
#[allow(bad_style)]
pub mod abbr_names {
    pub use super::BidiClass::LeftToRight as L;
    pub use super::BidiClass::RightToLeft as R;
    pub use super::BidiClass::ArabicLetter as AL;
}
/// Name bindings for the `BidiClass` property as they appear in Unicode documentation
#[allow(bad_style)]
pub mod long_names {
    pub use super::BidiClass::LeftToRight as Left_To_Right;
    pub use super::BidiClass::RightToLeft as Right_To_Left;
    pub use super::BidiClass::ArabicLetter as Arabic_Letter;
}
#[allow(bad_style)]
#[allow(unreachable_patterns)]
impl ::std::str::FromStr for BidiClass {
    type Err = ();
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "LeftToRight" => Ok(BidiClass::LeftToRight),
            "RightToLeft" => Ok(BidiClass::RightToLeft),
            "ArabicLetter" => Ok(BidiClass::ArabicLetter),
            "L" => Ok(BidiClass::LeftToRight),
            "R" => Ok(BidiClass::RightToLeft),
            "AL" => Ok(BidiClass::ArabicLetter),
            "Left_To_Right" => Ok(BidiClass::LeftToRight),
            "Right_To_Left" => Ok(BidiClass::RightToLeft),
            "Arabic_Letter" => Ok(BidiClass::ArabicLetter),
            _ => Err(()),
        }
    }
}
#[allow(bad_style)]
#[allow(unreachable_patterns)]
impl ::std::fmt::Display for BidiClass {
    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        match *self {
            BidiClass::LeftToRight => write!(f, "{}", "Left-to-Right"),
            BidiClass::RightToLeft => write!(f, "{}", "Right-to-Left"),
            BidiClass::ArabicLetter => write!(f, "{}", "Right-to-Left Arabic"),
            BidiClass::LeftToRight => write!(f, "{}", "Left_To_Right".replace('_', " ")),
            BidiClass::RightToLeft => write!(f, "{}", "Right_To_Left".replace('_', " ")),
            BidiClass::ArabicLetter => write!(f, "{}", "Arabic_Letter".replace('_', " ")),
            _ => {
                write!(
                    f,
                    "{}",
                    match *self {
                        BidiClass::LeftToRight => "L",
                        BidiClass::RightToLeft => "R",
                        BidiClass::ArabicLetter => "AL",
                        BidiClass::LeftToRight => "LeftToRight",
                        BidiClass::RightToLeft => "RightToLeft",
                        BidiClass::ArabicLetter => "ArabicLetter",
                    }
                )
            }
        }
    }
}
#[allow(bad_style)]
impl ::char_property::EnumeratedCharProperty for BidiClass {
    fn abbr_name(&self) -> &'static str {
        match *self {
            BidiClass::LeftToRight => "L",
            BidiClass::RightToLeft => "R",
            BidiClass::ArabicLetter => "AL",
        }
    }
    fn all_values() -> &'static [BidiClass] {
        const VALUES: &[BidiClass] = &[
            BidiClass::LeftToRight,
            BidiClass::RightToLeft,
            BidiClass::ArabicLetter,
        ];
        VALUES
    }
}
```

All three of the `abbr`, `long`, and `display` properties of the enum are optional, and have sane fallbacks: `abbr_name` and `long_name` return `None` if unspecified, and `fmt::Display` will check, in order, for `display`, `long_name`, `abbr_name`, and the variant name until it finds one to use (stringified, of course).

`FromStr` is defined, matching against any of the provided `abbr`, `long`, and variant name.

<hr />

Important notes:

- <strike>The current format uses associated consts, so it works on beta but won't work on stable until 1.20 is stable.</strike>
  - Consts have a slightly different meaning than `pub use` -- `pub use` aliases the type where `const` is a new object and if used in pattern matching is a `==` call and not a pattern match.
  - For this reason I'm actually slightly leaning towards using `pub use` even once associated consts land; they're compartmentalized (so `use Property::*` doesn't pull in 3x as many symbols as there are variants). After using the const based aliasing for a little bit, I'm inclined to like the current solution of `unic::ucd::bidi::BidiClass::*` + `unic::ucd::bidi::bidi_class::abbr_names::*`. These really should be a `pub use` and not a `const`.
  - Note that I still think `const` are the way to go for cases like `Canonical_Combining_Class`, though.
- <strike>The current syntax could easily be adapted to use modules instead of associated consts, but was written with the associated consts so we could get a feel of how it would look with them.</strike>
- The zero-or-more meta match before a enum variant conflicts with the ident match before 1.20. See rust-lang/rust#42913, rust-lang/rust#24189
- There only tests of the macro are rather thin and could be expanded.
- It's a macro, so the response when you stick stuff not matching the expected pattern is cryptic at best.
- The `CharProperty` trait is pretty much the lowest common denominator. It's a starting point, and we can iterate from there.
- How and where do we want to make `CharProperty` a externally visible trait? Currently having it in namespace is the only way to access `abbr_name` and `long_name`.
- <strike>Earlier discussion suggested putting these into `unic::utils::char_property`. Moving it would be simple, but for now it's living in the root of `unic-utils`</strike>
- <strike>The crate `unic-utils` is currently in the workspace by virtue of being a dependency of `unic`, but is not in any way visible a crate depending on `unic`.</strike>
- <strike>Documentation doesn't exist.</strike>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants