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

Syntactically permit postfix macros to reject them later #78849

Closed
wants to merge 2 commits into from

Conversation

est31
Copy link
Member

@est31 est31 commented Nov 7, 2020

Change the parser to syntactically permit postfix bang macros (foo.bar!()) to be parsed by rustc, but still reject them later on before they would be expanded or even resolved.

Similar prior work: #75857 and #66183.

The intention of this PR is to allow attribute macros to lower the postfix macro syntax to something of their choosing.

For everyone else, it improves error messages.

error: expected one of `(`, `.`, `::`, `;`, `?`, `}`, or an operator, found `!`
  --> $DIR/postfix-macros.rs:2:5
   |
LL |     "Hello, world!".to_string().println!();
   |                                        ^ expected one of 7 possible tokens

becomes

error: forbidden postfix macro call `println`
  --> $DIR/postfix-macros.rs:2:5
   |
LL |     "Hello, world!".to_string().println!();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^--------^^
   |                                 |
   |                                 macros can't be called postfix position

@rust-highfive
Copy link
Collaborator

r? @lcnr

(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 Nov 7, 2020
@jonas-schievink jonas-schievink added relnotes Marks issues that should be documented in the release notes of the next release. T-lang Relevant to the language team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. labels Nov 7, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 7, 2020

@est31
Copy link
Member Author

est31 commented Nov 7, 2020

@jyn514 I can't find an FCP for #75857 but #66183 had an FCP, so I guess one is needed. cc @joshtriplett who has authored an RFC about postfix macros.

@jyn514
Copy link
Member

jyn514 commented Nov 7, 2020

cc @ehuss too

@lcnr
Copy link
Contributor

lcnr commented Nov 7, 2020

r? @petrochenkov

@rust-highfive rust-highfive assigned petrochenkov and unassigned lcnr Nov 7, 2020
@petrochenkov
Copy link
Contributor

I'll reroute this to the lang team immediately.
This is a much larger change from the category "accepted syntactically, but always rejected semantically" than the previous precedents.
I think merging an RFC like rust-lang/rfcs#2442 and stating that "yes, we will accept postfix macros semantically sooner or later" is a pre-requisite for landing something like this.

@petrochenkov petrochenkov added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 7, 2020
@est31
Copy link
Member Author

est31 commented Nov 7, 2020

@petrochenkov that RFC won't get merged any time soon, but not because of disagreements on the syntax, but because of disagreement on the semantics. I haven't read a single comment in that thread which disagreed about the proposed invocation syntax. It's basically obvious how it should be.

As to the size of the change, the current diff is smaller by one line than #75857. So I wouldn't really say it's bigger :). Edit: I guess you meant semantic size, which is indeed larger for this change.

@lcnr
Copy link
Contributor

lcnr commented Nov 7, 2020

I feel that unlike #75857 this is at least partially an expression of intent, so I do believe it requires at least an FCP

This improves error messages and enables proc macros
to work with the postfix macro syntax.
@joshtriplett
Copy link
Member

joshtriplett commented Nov 8, 2020

While I would love to have postfix macros (I proposed RFC 2442 in the first place), I don't think we should parse them if we're not going to accept them.

I would love to see RFC 2442 revived. I've just rewritten that RFC to bring it up to date with current Rust.

Thank you for working on this, @est31!

@est31
Copy link
Member Author

est31 commented Nov 8, 2020

@joshtriplett thanks for the kind words and thanks for reviving the RFC. I hope that this time it gets through, but I'm doubtful, given the contents of the thread. Parsing postfix macros would allow experimentation with attribute macros outside of the main language.

@petrochenkov
Copy link
Contributor

The intention of this PR is to allow attribute macros to lower the postfix macro syntax to something of their choosing.

One more general solution to this problem is to delay parsing of anything inside delimited groups ((), [], {}) until expansion (#75820 (comment), #65860).

This way people will be able to use pretty much any arbitrary syntax (modulo the the requirement on balanced delimiters) under #[cfg(FALSE)] or in proc macro input.

@est31
Copy link
Member Author

est31 commented Nov 8, 2020

@petrochenkov that'd be a great idea IMO. The main reason these syntax extensions are proposed are attribute macros. Most use cases would benefit tremendously from being able to do a lot of things inside delimitered groups. Would help my proposal and also would have helped #66183, but not #75857 as the attr is directly on the module. I'm not sure about formatters, but imo it should be easy for formatters to speculatively treat something as Rust and if it isn't then just bail out somehow.

@joshtriplett
Copy link
Member

joshtriplett commented Nov 9, 2020

@petrochenkov That seems like a great idea for many reasons, not least of which improving handling of conditional compilation so that it works to hide new features from old compilers.

@est31
Copy link
Member Author

est31 commented Nov 16, 2020

Closing in favour of a more general approach: #78849 (comment)

@est31 est31 closed this Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-fcp This change is insta-stable, so needs a completed FCP to proceed. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-lang Relevant to the language 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