Clearer error messages when parser encounters an outer attribute when parsing inner attributes.

[brson]

Consider this example:

/**
 * My mod
 */

#![recursion_limit = "100"]
fn main() {}

Produces:

error: an inner attribute is not permitted in this context
 --> <anon>:5:3
  |>
5 |> #![recursion_limit = "100"]
  |>   ^
help: place inner attribute at the top of the module or block

error: aborting due to previous error

Compilation failed.

It's quite impossible to understand what's going on if you don't know that /** */ is an outer attribute. Unfortunately after looking at the parser a bit it's not obvious what to do to make this situation clearer. Possibly parse_outer_attributes should emit a new error if its call to to parse_attribute fails. It can look at whether it has a previously-parsed attribute, and whether it was a doc comment, and emit better errors, e.g.

For non-doc comments: "inner attributes may not follow outer attributes"; for doc comments: "inner attributes may not follow outer doc-comments". A good comparison to draw is with the existing error in parser_outer_attributes when it encounters a incorrect inner doc comment. The errors should be similar for incorrect inner attributes vs. incorrect inner doc comments.

Original issue.

[brson]

I marked this easy assuming my solution above is workable, but it'll still take some hacking to figure out.

[aravind-pg]

I've looked into this a little, and it's not obvious how to handle this in a clean and non-invasive way. But how about the following approach?

• parse_attribute currently takes in a boolean permit_inner that says whether the context in which it's being called permits inner attributes.
• Change this parameter to instead be an enum, say InnerAttributeParsePolicy, that basically allows us to attach an optional reason (as a string for now) why we're not allowing inner attributes. Something of the following form:

enum InnerAttributeParsePolicy {
  Permitted,
  // the string specifies the reason in the form of
  // a recommended error message, perhaps
  NotPermitted(Option<&str>)
}

Alternatively to keep things simple for now:

enum InnerAttributeParsePolicy {
  Permitted,
  NotPermitted,
  NotPermittedFollowingDocComment
}

• And now like you suggested, in parse_outer_attributes, before calling parse_attribute we check what the last attribute parsed was. If it was a doc comment, set the reason appropriately, otherwise leave it None.

Overall this approach doesn't look too horrible, I feel (we can bikeshed names freely later), and it has the benefit of being pretty extensible. Would really appreciate high-level feedback on the overall idea before diving into this though, being a complete beginner.

[brson]

@aravind-pg Yes, that looks like it will work! 👍