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

Allow parsing foreign impl blocks #102376

Closed
wants to merge 1 commit into from

Conversation

maurer
Copy link
Contributor

@maurer maurer commented Sep 27, 2022

Allow parsing foreign impl blocks for use by proc macros.

This allows more ergonomic description of methods and associated functions both when describing rust types to be exported and when importing class-like objects from other languages.

My primary motivation for this change is clean support for C++ static methods - this could be done with alternate syntax (e.g. explicit attributes to associate these functions with classes), but this seems much cleaner and potentially useful for interfacing with other languages beyond C++.

The Rust language itself remains unmodified - foreign impl blocks which make it past the macro expansion stage will not compile, and additional tests have been added to ensure this remains the case.

#75857 is prior art on this kind of change, but is substantially less involved.

cc @dtolnay

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 27, 2022
@rust-highfive
Copy link
Collaborator

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 27, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 27, 2022

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

@rust-log-analyzer

This comment has been minimized.

@maurer maurer force-pushed the permit-foreign-impl branch from 9c3c503 to 375246a Compare September 27, 2022 19:52
@rust-log-analyzer

This comment has been minimized.

@maurer maurer force-pushed the permit-foreign-impl branch from 375246a to 59a6552 Compare September 27, 2022 20:37
@rust-log-analyzer

This comment has been minimized.

Allow `impl` blocks in foreign modules for more ergonomic use in macros.
`impl` blocks still have no meaning in foreign modules and will cause an
error if still present during lowering.

Also adds explicit tests for the previous similar foreign module macro
extensions to ensure that neither this nor those are accepted or crash
the compiler when *not* fed to a macro.
@maurer maurer force-pushed the permit-foreign-impl branch from 59a6552 to f728939 Compare September 27, 2022 21:42
@est31
Copy link
Member

est31 commented Sep 28, 2022

I'm not sure they'll accept this, given that #78849 was rejected. Maybe one could generally make parsing weaker for attr proc macros: #78849 (comment)

Edit: obviously good C++ compatibility is more important than postfix macros, but the general argument of not adding even more exceptions to the tokens, and this seems to be a big exception, still exists.

@nagisa
Copy link
Member

nagisa commented Sep 28, 2022

cc @petrochenkov

I’m thinking that this probably wants an compiler team MCP with all the votes and such before this can land.

@bors
Copy link
Contributor

bors commented Sep 29, 2022

☔ The latest upstream changes (presumably #102461) made this pull request unmergeable. Please resolve the merge conflicts.

@nagisa nagisa added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 28, 2022
@JohnCSimon
Copy link
Member

@maurer
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Jan 1, 2023
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Jan 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants