Incorrect and inconsistent jointness of tokens in desugared doc comment

[dtolnay]

I have not minimized this yet but in TeXitoi/structopt#88 we are seeing inexplicable behavior when iterating over tokens of a struct field doc comment. Related to #49545 so mentioning @alexcrichton.

One of their test cases contains the following struct.

/// Lorem ipsum
#[derive(StructOpt, PartialEq, Debug)]
struct LoremIpsum {
    /// Fooify a bar
    /// and a baz
    #[structopt(short = "f", long = "foo")]
    foo: bool,
}

Within their macro implementation we are seeing the desugared doc comment of /// Fooify a bar having an Alone spacing:

Op(Op { op: '=', spacing: Alone, span: Span(Span { lo: BytePos(0), hi: BytePos(0), ctxt: #0 }) })
Literal(Literal(Literal(Str_(/// Fooify a bar), None)))

while the desugared /// and a baz has a Joint spacing. I believe the Joint is incorrect because only an Op followed by another Op should be able to have Joint spacing.

Op(Op { op: '=', spacing: Joint, span: Span(Span { lo: BytePos(0), hi: BytePos(0), ctxt: #0 }) })
Literal(Literal(Literal(Str_(/// and a baz), None)))

I stuck the following loop at the top of their derive entry point:

    for tt in input.clone() {
        println!("{:#?}", tt);
    }

and it indicates that the first doc comment has kind: Tree while the second has kind: JointTree.

                        TokenStream {
                            kind: Tree(
                                Token(
                                    Span {
                                        lo: BytePos(
                                            0
                                        ),
                                        hi: BytePos(
                                            0
                                        ),
                                        ctxt: #0
                                    },
                                    DocComment(
                                        /// Fooify a bar
                                    )
                                )
                            )
                        },
                        TokenStream {
                            kind: JointTree(
                                Token(
                                    Span {
                                        lo: BytePos(
                                            0
                                        ),
                                        hi: BytePos(
                                            0
                                        ),
                                        ctxt: #0
                                    },
                                    DocComment(
                                        /// and a baz
                                    )
                                )
                            )
                        },

[alexcrichton]

@dtolnay I think I know what's happening here in terms of where the Joint is coming from but I'm also a little confused as to where TeXitoi/structopt#88 is happening.

I was unable to get JointTree or Joint to show up though unfortunately in a smaller hello-world test with just adding some doc comments.

I'll see what I can about the spurious Joint at least though

[TeXitoi]

The bug has nothing related to the PR, that's just the first time the error is reported. I can reproduce the failed assertion on master.

But there is a description of what happen in the PR. Basically, there is a 2 line doc comment, but only the first one is processed.

[TeXitoi]

https://github.com/TeXitoi/structopt/blob/master/tests/doc-comments-help.rs first test.

[dtolnay]

Yeah I couldn't reproduce this in a straightforward minimized macro either, but checking out the structopt PR at commit 5000840d1a6b79cf704caa927b4f5b15d29978c9 and cargo test --features nightly does reproduce it. I am curious what you find!

Their problem is here + here where a Joint doc = "..." is not considered a legal Meta.

[alexcrichton]

Ok thanks for the info! I've confirmed that this is fixed by #49597 where I jiggered things around a bit. Namely it appears that the Alone spacing is indeed what we need to fix this.

[dtolnay]

Repro script

#!/bin/sh

cargo new --lib structopt_derive
cargo new --lib repro

echo >structopt_derive/src/lib.rs '
#![feature(proc_macro)]

extern crate proc_macro;
use proc_macro::{TokenStream, TokenTreeIter, TokenTree, TokenNode, Spacing};

#[proc_macro_derive(StructOpt)]
pub fn derive_structopt(input: TokenStream) -> TokenStream {
    let mut iter = input.into_iter();
    assert_eq!(iter.next().unwrap().to_string(), "struct");
    assert_eq!(iter.next().unwrap().to_string(), "S");
    let mut inner = unwrap_group(iter.next().unwrap());

    assert_eq!(inner.next().unwrap().to_string(), "#");
    let mut x = unwrap_group(inner.next().unwrap());
    assert_eq!(x.next().unwrap().to_string(), "doc");
    println!("{:?} {}", unwrap_spacing(x.next().unwrap()), x.next().unwrap());

    assert_eq!(inner.next().unwrap().to_string(), "#");
    let mut y = unwrap_group(inner.next().unwrap());
    assert_eq!(y.next().unwrap().to_string(), "doc");
    println!("{:?} {}", unwrap_spacing(y.next().unwrap()), y.next().unwrap());

    TokenStream::empty()
}

fn unwrap_group(tt: TokenTree) -> TokenTreeIter {
    match tt.kind {
        TokenNode::Group(_, s) => s.into_iter(),
        _ => unimplemented!(),
    }
}

fn unwrap_spacing(tt: TokenTree) -> Spacing {
    match tt.kind {
        TokenNode::Op(_, s) => s,
        _ => unimplemented!(),
    }
}
'

echo >>structopt_derive/Cargo.toml '
[lib]
proc-macro = true
'

echo >repro/src/lib.rs '
#![allow(dead_code)]

#[macro_use]
extern crate structopt_derive;

fn f() {
    #[derive(StructOpt)]
    struct S {
        /// X
        /// Y
        #[doc(hidden)]
        foo: bool,
    }
}
'

echo >>repro/Cargo.toml '
structopt_derive = { path = "../structopt_derive" }
'

cargo build --manifest-path repro/Cargo.toml

Output

Alone "/// X"
Joint "/// Y"

[dtolnay]

Interestingly in the script if you move struct S outside of the fn f, then the output is correct.

Alone "/// X"
Alone "/// Y"

@alexcrichton when you say "jiggered things around a bit" is that of the intentional sort or the accidentally-fixed-it sort? It would be good to understand why the token stream was different depending on whether the macro is invoked inside a function or outside -- not clear in your PR what might have fixed that.

[alexcrichton]

@dtolnay sure yeah, worth documenting!

So I'm not really sure why, but the JointTree vs Tree is what's going on here. For whatever reason rustc seems to be parsing a JointTree inside a function and a Tree outside, I'm not really sure why. Assuming that though we know that the is_joint is true for the second macro. The is_joint is then later used for spacing in Op tokens, and in the op! macro you'll see how the last token uses the op_kind and all previous ones are Spacing::Joint.

Unfortunately I fat-fingered this a bit and in the doc comment expansion the code also uses op! as a sort of shortcut to TokenTree::Op. This is the bug, however, as the tokens are inheriting the is_joint variable and Joint spacing accidentally. The internal tokens don't have the same spacing, and we don't actually have any ability to communicate a Joint doc comment so that information needs to get lost (instead of preserved at a sort of random location).

In the PR I made these are both explicitly changed to Spacing::Alone as they're supposed to be (as the operators aren't actually joint with anything).

[dtolnay]

For whatever reason rustc seems to be parsing a JointTree inside a function and a Tree outside, I'm not really sure why.

This is really what I was interested in. I filed #49604 to follow up. Thanks!