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

Trailing doc comments in trait definition causes an error #4106

Closed
brendanzab opened this issue Dec 4, 2012 · 22 comments
Closed

Trailing doc comments in trait definition causes an error #4106

brendanzab opened this issue Dec 4, 2012 · 22 comments
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR) A-grammar Area: The grammar of Rust A-parser Area: The parsing of Rust source code to an AST

Comments

@brendanzab
Copy link
Member

trait_comments.rs

trait A {
    fn a();     /// doc comment
}

fn main() {}

error:

$ rustc trait_comments.rs 
trait_comments.rs:3:0: 3:1 error: unexpected token: `}`
trait_comments.rs:3 }

It seems to only be triple slashes that cause it - // comment compiles fine.

Edit: Interestingly it seems that this compiles fine:

trait A {
    fn a();     /// comment
    fn b();     // comment
}

fn main() {}
@Dretch
Copy link
Contributor

Dretch commented Dec 4, 2012

In the first example the doc-comment comes after fn a() - even though it is on the same line. Doc-comments need to come before the item they apply to (except for inner doc-comments, which come inside their item).

// ... type comments, with only two slashes, are not doc-comments (meaning rustdoc ignores them), so the parser doesn't need to know which item they apply to.

The problem here is that the error message when doc-comments are used in places they don't belong is not very insightful.

@Dretch
Copy link
Contributor

Dretch commented Dec 4, 2012

I will try and have a go at improving the error message, since this keeps coming up.

@brendanzab
Copy link
Member Author

But seeing as it's a comment, shouldn't it pass the compilation ok? I wasn't generating any documentation here...

@Dretch
Copy link
Contributor

Dretch commented Dec 5, 2012

That argument does make some sense.

On the other hand, if rustc didn't complain then you would likely never had
noticed that you were putting doc-comments in the wrong places - and when
you did eventually run rustdoc it would have assigned your comments to the
wrong items (or in some cases given you an error).

@brendanzab
Copy link
Member Author

Oh I see. Re-reading the rustdoc article, it seems like /// is actually just sugar that expands to a doc attribute.

With that 'insider' knowledge, I now understand why this gives an error:

fn main() {}

/**
*/

A scenario in which this would crop up would be:

fn main() {}

/**
 * Doc comment for foo
 */
// fn foo() {
//     io::println(~"foo")
// }
$ rustc rustdoc_breakage.rs
rustdoc_breakage.rs:3:1: 6:0 error: expected item
rustdoc_breakage.rs:3 /**
rustdoc_breakage.rs:4  * Doc comment for foo
rustdoc_breakage.rs:5  */
rustdoc_breakage.rs:6 // fn foo() {

I unwittingly ran into this a few weeks ago, and I've only just figured out the cause. It's very confounding if you have no idea what to look for. I would have thought that above all, the rule that comments should be ignored at compile time should be sacrosanct. If at all possible, the error should occur in rustdoc, not rustc.

@brson
Copy link
Contributor

brson commented Dec 5, 2012

This is confusing behavior.

At the time we were hoping that it would be useful to know at compile time that your rustdoc didn't actually apply to anything. It has helped me a few times, particularly when I use an outer rustdoc at the top of a file, like /** */, but I should be using /*! */.

I agree though that it would be better for this to be a rustdoc error instead of a rustc error, but there are complications.

The reason it's like this is because rustdocs are actually discovered in the lexer, and converted to attributes so that the parser can associate them with items (or whatever) just like any attribute. Otherwise you have to have some heuristic that later extracts the doc comments and tries to associate them with code.

One very nice thing that parsing rustdocs into attributes allows us to do is put the docs directly into the crate metadata. In theory we should be able to extract docs from existing, compiled crates, and also get docs at runtime. If we stopped parsing rustdocs at compile time we would lose that property.

One possibility is to put a flag on the lexer to control whether it extracts docs. Under normal compilation it would ignore them, but rustdoc would lex the docs. In the future, when it is actually useful, crates could add a #[embed_docs] attribute to turn on doc lexing.

@nikomatsakis has expressed opinions about this before.

@brson
Copy link
Contributor

brson commented Dec 5, 2012

We could also just try to improve the error messages.

@Dretch
Copy link
Contributor

Dretch commented Dec 5, 2012

@brson

Otherwise you have to have some heuristic that later extracts the doc comments and tries to associate them with code.

Would this be so bad?

The parser could intercept DOC_COMMENT tokens from the lexer and push them onto some list for later processing by rustdoc.

The heuristic would be to simply look for the following/preceding (for outer/inner comments) item based on character position in the file. There might be some special rules to allow things like

fn a(); /// doc comment on same line.

Rustdoc could then emit error messages like:

failed to find associated item for doc-comment at line:col ....

@brson
Copy link
Contributor

brson commented Dec 5, 2012

@Dretch I am not sure whether it would be bad or not. It is harder. If we went in that direction we should remove doc comments from the lexer. We already have code that can extract all comments from source files for the pretty printer to use, so rustdoc could just dig through those for doc comments.

The big win we could get from that approach would be to remove 'inner doc comments' that use !, instead just making a guess based on the position of the comment and surrounding context about what the comment applies to. I do think this is how most people expect doc comments to work.

@brendanzab
Copy link
Member Author

Have you guys also considered a doxygen-style ///< ... or /**< ... */ for trailing doc comments that are put before the preceding item? This very useful in my case.

@brson
Copy link
Contributor

brson commented Dec 6, 2012

@bjz agreed that it's useful. The reason we don't have them is because they don't fit the attribute model of either being 'outside' (and before) the thing it applies to or 'inside'.

fn foo(
  #[outer_attribute_can_go_here]
  bar: baz
  // no attribute can go here
) { }

@brson
Copy link
Contributor

brson commented Dec 6, 2012

Switching to the heuristic model would of course make ///< work.

@brson
Copy link
Contributor

brson commented Dec 6, 2012

Oh, also I'm not sure if function arguments actually accept attributes like my example indicates.

@brendanzab
Copy link
Member Author

Hah, I just thought of this:

fn generic_function
<
    T,      ///< Something
    U       ///< Something else...
> (
    a: T,   ///< An argument
    b: U    ///< Another argument
) -> U {
    ...
}

struct Foo {
    a: A,    ///< blah
    b: B,    ///< blah blah
}

Might get a little unreadable though :P

Maybe rustdoc could give an error here?

/// blah
const foo: int = 0; ///< blah
const bar: int = 1; ///< blah

'Conflicting doc comments at line 1 and 2' or something.

Anyway, I'm not the one implementing this, and it looks pretty complex to figure out. But yeah, I can dream... :)

@brendanzab
Copy link
Member Author

Oh, and another thing. Do you currently do an error for this?

/**
 * ...
 */
/// ...
fn blah() { }

@brson
Copy link
Contributor

brson commented Dec 13, 2012

@bjz I believe rustdoc will concatenate all doc comments on a single item into one.

@AndresOsinski
Copy link

I just ran into accidentally putting a doc-comment at the end of a struct definition. Is there any way that the error message could be improved?

@graydon
Copy link
Contributor

graydon commented Jun 20, 2013

Reproduced 2013-06-19. Doesn't strike me as terribly urgent, but at very least the error message could do with some improvement.

@huonw
Copy link
Member

huonw commented Aug 5, 2013

Triage 2013-08-05, still an issue. I guess the error message could be/contain trailing doc comment; must precede an item (an similarly trailing attribute; ... for trait A { #[foo] }).

@emberian
Copy link
Member

Untagging rustdoc, it's purely a rustc issue. I really recommend against pushing the hueristics around. THe explicit behavior is very nice.

@lilyball
Copy link
Contributor

Perhaps this could be turned into a warning instead of a hard error?

@huonw
Copy link
Member

huonw commented Aug 19, 2013

This seems to be a sub-bug of #2789. Closing; feel free to swap the open/closed statuses if someone feels very strongly that this is the "correct" bug and the other one should be closed.

@huonw huonw closed this as completed Aug 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR) A-grammar Area: The grammar of Rust A-parser Area: The parsing of Rust source code to an AST
Projects
None yet
Development

No branches or pull requests

8 participants