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

dyn/impl syntax (incl. old dyn-less trait objects) doesn't always allow trailing +. #1075

Closed
eddyb opened this issue Oct 3, 2021 · 3 comments

Comments

@eddyb
Copy link

eddyb commented Oct 3, 2021

Except for the case in #120, that #265 fixed, and a similar one with explicit dyn, there doesn't seem to be general support for trailing + in a type (after a path).

That is, Trait+, dyn Trait+ and impl Trait+ are all valid Rust types, but syn doesn't accept them.

This playground contains examples of the syntax being accepted by Rust, and also the failure of syn to parse the input.


This was discovered by creduce (likely) removing the RHS of the +, while doing an automated reduce (rust-lang/rust#89195 (comment)).
I tried using rust-reduce on the output of creduce (because of rust-lang/rust#89195 (comment)), and it failed with "expected identifier" (because it relies on syn). However, once rustfmt was ran on that file, rust-reduce was able to work again.

To track down the exact bug that was being found, creduce --not-c script.sh current.rs was used with:

#!/usr/bin/env bash

rustc -Z unpretty=normal current.rs > /dev/null || exit 50

timeout 1 rust-reduce true <(cat current.rs) 2>&1| grep "expected ident"

as script.sh, resulting in this reduction, after 65 seconds (down from a 13856 byte file):

mod a {
  mod b {
    impl c { fn d()->e + ; }
  }
}
@SNCPlay42
Copy link

SNCPlay42 commented Oct 4, 2021

See also #1073. I think #1074 only fixed this for impl Trait, not dyn Trait/bare trait objects.

(Generic bounds/where clauses, type alias bounds and trait supertraits with a final + are all parsed correctly).

@dtolnay
Copy link
Owner

dtolnay commented Oct 6, 2021

I published 1.0.79 fixing the dyn Trait + and Trait + cases. The impl Trait + was fixed previously in #1074.

I would be shocked if this is the only thing in the way of reliably parsing creduce-generated programs, but at least it's a step.

@dtolnay dtolnay closed this as completed Oct 6, 2021
@eddyb
Copy link
Author

eddyb commented Oct 6, 2021

Thanks!

I would be shocked if this is the only thing in the way of reliably parsing creduce-generated programs

Heh, I haven't ran into anything else yet. creduce likes to remove extraneous tokens, so I believe I only got into trouble with this because I had to stop creduce partway through (to change the script to cargo check).

(Of course there's still a concern for anything that uses syn while the reduction is ongoing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants