Allow #![] attributes to work in 0.16

[wycats]

In 0.16, the default mode for lalrpop will import the generated module
through a macro that include!s the contents.

PR #338 made it possible to use include! with vanilla modules by
moving all of the built-in whole-module attributes to individual
modules. However, #115 places user-generated outer attributes in the
same problematic location.

This PR moves user-supplied outer attributes to the same location as
the fix in #338.

[nikomatsakis]

I have some concerns about this change -- it feels surprising to emit the module attributes also on the internal modules. Maybe we should add some kind of explicit Edition support, if that is the main motivation?

[nikomatsakis]

we don't usually edit lalrpop-snap

[nikomatsakis]

Something feels fishy about this. This attribute is going to apply to the next module thing that gets emitted ... oh, I see, and the for loop below is intentionally converting #![foo] into #[foo]?

[nikomatsakis]

OK, re-reading your comment, I think I understand better. So there are two distinct things going on here:

• Converting #![foo] to #[foo] -- this may indeed be a good idea, given the problems around #![] attributes and include! (IIUC)
• Apply those attributes to all the generated submodules -- I'm less sure about this, but it may make sense? I'm not sure if there was a direct motivation for this.

[nikomatsakis]

Ah, probably we apply the attributes to the submodules just because it's not obvious how to apply them .. otherwise?

[Marwes]

Ah, probably we apply the attributes to the submodules just because it's not obvious how to apply them .. otherwise?

Yep. rust-lang/rust#18810

[Marwes]

Thinking about it, perhaps we could wrap the generated module as

mod name {
    #![attr]
     // Rest of the code goes here
}
pub use name::{Parser, ..};

[alercah]

Personally I think I'm inclined to the mod_path! workaround, because it allows attaching other attributes from the outside too:

#[deny(...)]
lalrpop_mod!(...);

[alercah]

Oh, nevermind, this is already supported, just with slightly different syntax. But still, I think that's the more elegant solution anyhow.

[alercah]

Would you be interested in a pull request for this?

[franleplant]

Hi guys! I have a question, #115 allegedly allowed attributes in top of the grammar file, I tried with 0.16.x and it doesnt work, and then I see this PR.

From what I understand, and, as a nice recap for future people, did this feature broke on 0.16.x and that is why this PR exists?

[franleplant]

@nikomatsakis how about adding by default a #[allow(clippy::all)] to the synthesized module?

[NyxCode]

For everyone who stumbled upon this issue:
the lalrpop_mod! macro can accept attributes like this: lalrpop_mod!(#[allow(clippy::all)] grammar);

[nikomatsakis]

Thanks for the PR. We're going to reconsider this as we move towards 1.0, but deferring for now.