-
Notifications
You must be signed in to change notification settings - Fork 13k
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
crate-ify and delete unused code from syntax::parse #51265
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
9ade7c8
to
7ff11e3
Compare
This comment has been minimized.
This comment has been minimized.
7ff11e3
to
0b4f8e8
Compare
r? @jseyfried @petrochenkov or @pnkfelix |
Thanks! |
📌 Commit 0b4f8e8 has been approved by |
🔒 Merge conflict |
☔ The latest upstream changes (presumably #51068) made this pull request unmergeable. Please resolve the merge conflicts. |
0b4f8e8
to
60058e5
Compare
@bors r=petrochenkov |
📌 Commit 60058e5 has been approved by |
…henkov crate-ify and delete unused code from syntax::parse This is intended primarily to ensure the compiler catches dead code for us in more cases.
☀️ Test successful - status-appveyor, status-travis |
Is it possible to make some metods public again? I find that the changes introduced in this PR break a few number of tools and libraries that depend on libsyntax. |
Hm, I somewhat expected that tools/libraries like Rocket should mostly be using proc macro's these days, not libsyntax directly. If there's a list of methods/types that need to be made public, I'd be happy to review a PR which makes them pub again. |
…ochenkov Make parse_ident public `parse_ident` was made private in rust-lang#51265. In rustfmt the method is used to create a custom parser for macro call.
…Simulacrum Make parse_seq_to_end and parse_path public (see rwf2/Rocket#660, rust-lang#51265) Rocket currently uses `parse_seq_to_end` and `parse_path` in its codegen macros. Assuming I tested correctly, this is the minimal set of methods that are currently necessary to build Rocket again. I would be happy to add documentation of this and Rocket's other usages, if desired.
span_fatal and parse_block were made private in rust-lang#51265. These methods are used in stainless. Related rust-lang#51498 rust-lang#51504
Make parse_ident public `parse_ident` was made private in #51265. In rustfmt the method is used to create a custom parser for macro call.
…Simulacrum Make parse_seq_to_end and parse_path public (see rwf2/Rocket#660, rust-lang#51265) Rocket currently uses `parse_seq_to_end` and `parse_path` in its codegen macros. Assuming I tested correctly, this is the minimal set of methods that are currently necessary to build Rocket again. I would be happy to add documentation of this and Rocket's other usages, if desired.
What about unstable proc macros that relied on these fns? |
I believe the expectation is that even unstable proc macros should be using syn/quote or other libraries for the purpose of parsing Rust code and generating Rust code; they should not be using libsyntax. That is, proc macros shouldn't use |
@Mark-Simulacrum Thanks for the quick response. It's been tough to follow the state of proc macros, so I haven't updated things in a while. Last time I tried I ran into cross-crate problems, and the bug I filed, #44817, hasn't been fixed yet. Do you mind giving me a quick pointer to what I should be doing right now? |
I don't believe that there's a way to fix #44817 with proc macros quite yet, and I believe we're unprepared to stabilize anything "hygienic" at this point. For the time being, if you want to fix the issue opened on tarpc I'll approve PRs that re-pub the relevant APIs; long term I imagine there will be a better solution for you. I also haven't looked at how the syntax extensions in tarpc are implemented, it's possible that we already have sufficient functionality for them. If you want, you might be able to get people more familiar with that feature to help on internals or IRC. |
@Mark-Simulacrum I tried to get it working with syn but am running into issues. I think the main problem is actually that things like |
It's probably best to move this to internals.rust-lang.org, but that feels odd to me. I'd expect syn to be able to parse proc macro input -- including $crate, if that's possible in proc macros. |
Ok, so the real problem -- which doesn't seem to have changed since I last tried to move off of plugins -- is that I can't call a proc_macro from inside a macro_rules macro.
I tried to reexport that proc macro from the tarpc crate root, but doing so doesn't make it visible to the macros declared in tarpc. If there's no way to do this, then I can't migrate to proc_macro at this time, and would prefer to roll back this PR. |
Re-pub some items whose visibilities were recently reduced. Reasons described in the most recent comments of #51265. tarpc can't move off of plugins until proc macros can be reexported from other crates. Fixes google/tarpc#191
This is intended primarily to ensure the compiler catches dead code for us in
more cases.