Change handling of doc comments to warn when mis-parsed, not error.

[brson]

Now that we've implemented doc comments as attributes I am doubtful that it was the correct thing to do. There are a lot of positions where attributes aren't valid and now the parser rejects things like:

let x = y; //// xy

I was planning putting some smarts into the lexer to detect four slashes and not consider those doc attributes, but then patrick checked in some code like ///< comment which was rejected.

Comments not being treated as whitespace is weird.

[brson]

On second thought, I kind of want to see how the current arrangement works before changing it. There is some amount of value in catching incorrect doc comments, which is sort of what happened with the ///< comment.

[brson]

I've run into another case where sections were formatted like /****** Some Stuff ******/. It failed to compile with an error that wouldn't lead you to expect that the comment was incorrect.

[catamorphism]

Nominating for "maturity #4 - well covered"

[graydon]

It represents a potential backwards incompatibility, so belongs on the backward compatible maturity milestone (or closed as wontfix)

[emberian]

I don't see how this could be a backwards incompatibility. Fixing the error to be better is no problem and allowing this behavior wouldn't break any valid code.

Nominating for production ready.

[emberian]

Example errors:

foo.rs:6:24: 7:0 error: expected item after attributes
foo.rs:6     let x = Foo{x: 5u}; /// bar
foo.rs:7 }

foo.rs:5:0: 6:0 error: expected item after attributes
foo.rs:5 /****** Some Stuff ******/
foo.rs:6     let x = Foo{x: 5u};

Which honestly isn't that bad but it's not good either.

[huonw]

Triage visit; all the examples reproduce.

I'm absorbing #4106 into this bug (#4106 has a lot of historical discussion).

[pzol]

Visiting for triage, still broken.

[mikedilger]

First off, I LOVE you guys and am so glad rust is happening. But I'm frustrated today about this issue. I'm still very new to rust, and it took me hours to figure out the problem, mostly because the error message I get is very misdirecting:

  mymod.js:6:3: 6:4 error: an inner attribute is not permitted in this context
  mymod.js:6 #![feature(phase)]

I strongly recommend a better error message, if at all possible, so you don't lose new people to the community.

My code was something like this:

/**
 * My mod
 */

// For log related macros:
#![feature(phase)]
#[phase(syntax, link)] extern crate log;

fn main() {
  error!("We have logging!");
}

[caipre]

I'll take a look at this.

[Manishearth]

What if we conditionally convert them to attributes if they are correct, else leave them as a comment and throw a warning?

[carols10cents]

(Hi, I'm trying to help make E-easy issues more accessible to newcomers 😸)

Checking reproducibility:

• Four slashes compiles without error
• The /****** Some Stuff ******/ comment compiles without error
• Three slashes does not compile with the error:

<anon>:3:16: 3:33 error: expected item after doc comment
<anon>:3     let x = 3; /// three slashes
                        ^~~~~~~~~~~~~~~~~

which seems clearer than the "expected item after attributes" error, so I think this is fixed

• [?] mikedilger's code does not compile with the same error reported:

<anon>:6:3: 6:4 error: an inner attribute is not permitted in this context
<anon>:6 #![feature(phase)]
           ^
<anon>:6:3: 6:4 help: place inner attribute at the top of the module or block
error: aborting due to previous error

I'm not totally convinced this is the same problem as the others though... but maybe it is? A smaller example:

/**
 * If you remove this comment, the compiler error will go away.
 * It's not very clear from the error message what the problem is.
 */

#![feature(phase)]
fn main() {
    println!("hi");
}

From #4106:

• [?] Original example fails with a similar confusing error as originally reported, that is, it's not clear that the comment is the problem, it just says "expected one of const, extern, fn, type, or unsafe, found }"
• Trailing empty doc comment fails with the clear error message "expected item after doc comment"

So it looks like there are still some less-than-ideal cases, and that the fix would be a better error message indicating that there was a doc comment in an unsupported location? Is that correct?

If so, where would one start looking to go about making the error message better for the remaining cases?

[Manishearth]

Thanks! 😄

[Manishearth]

and that the fix would be a better error message indicating that there was a doc comment in an unsupported location? Is that correct?

Yes. We basically want to add a note around https://github.com/rust-lang/rust/blob/master/src/libsyntax/parse/parser.rs#L3415

It would be even better if these were parsed correctly, but that's not an easy fix.

[wafflespeanut]

I'll try this! :)

[cflewis]

Should this be closed?

[mikedilger]

My comment above is still applicable, 2 years on. If it has been determined to be a separate issue, I can file it separately. Otherwise I think this should stay open.

[brson]

I've opened a new issue specifically to deal with @mikedilger's issue: #34516