Avoid stringifying and reparsing a TokenStream

[Aaron1011]

This PR changes removes uses of the pattern syn::parse_str(tokens.to_string()),
which loses Span information. In addition to making error messages
less precise, it can break code that depends on the hygiene information
carried by Span. Instead, syn::parse_quote is used to parse a
TokenStream into the desired AST struct.

I've added a test which demonstrates one of the problems with
the previous code - using $crate would previously cause
a panic, but now compiles successfully.

[Aaron1011]

Note that this PR might break some users of this crate.

[JelteF]

Thanks for the PR! Do you have an example of code that doesn't work now?

[Aaron1011]

@JelteF: The code I added to tests/lib.rs will cause a panic without this PR. I haven't found an example of code that fails to compile on the latest nightly without this change, but I can try to construct one if you'd like.

[Aaron1011]

I'm not sure why the macOS jobs failed - it looks like they didn't even start?

[Aaron1011]

@JelteF: Assuming that this PR gets merged, could you make a new point release afterwards? I'm planning to merge a Rust PR (rust-lang/rust#77153) soon, which will expose additional issues caused by throwing away hygiene information.

[JelteF]

What I don't understand is why you say this change can break existing code. It looks like it can only fix problems. I'm mainly asking, because I would like to know if this change is fine for a patch release or if it requires a bigger version bump. So could you explain your comment about this breaking existing users a bit better?

…

On Tue, 29 Sep 2020, 22:35 Aaron Hill, ***@***.***> wrote: @JelteF <https://github.com/JelteF>: Assuming that this PR gets merged, could you make a new point release afterwards? I'm planning to merge a Rust PR (rust-lang/rust#77153 <rust-lang/rust#77153>) soon, which will expose additional issues caused by throwing away hygiene information. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#144 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAI3YJT2YR6EEXU4BKFIKDDSIJAJDANCNFSM4R5BQASQ> .

[Aaron1011]

@JelteF: In general, preserving hygiene can break crates that were incorrectly relying on certain path being accessible. For example, there have been several bugs in rustc that incorrectly allowed code like this to compile:

macro_rules! use_it {
    ($code:block) => {
        let a = "25";
        $code
    }
}

fn main() {
    use_it!({
        let b = a;
    });
}

When the compiler started properly preserving hygiene, the function main() stopped compiling, since a is not supposed to be accessible in the let a = b; statement.

This PR isn't addressing a compiler bug, but the underlying principle is the same.

In this particular case, the only things being stringified are types (it looks like they get used in generic bounds). Currently, the only stable types of hygiene are call-site and mixed-site hygiene (see https://doc.rust-lang.org/beta/proc_macro/struct.Span.html#method.call_site and https://doc.rust-lang.org/beta/proc_macro/struct.Span.html#method.mixed_site), which means that there can't be multiple types at the same path, but with different hygiene. In the context of derive_more, this means that the only thing that should change is how crate and $crate are handled.

TL;DR - I was being overly cautious. The only thing that this could possibly break is code using the unstable Span::def_site() hygiene, which likely still has bugs. I think this should be good to merge.

[JelteF]

Released as 0.99.11, good luck with your rustc PR.