Skip to content

Conversation

pnkfelix
Copy link
Contributor

@pnkfelix pnkfelix commented May 11, 2021

Re-add support for parsing (and pretty-printing) inner-attributes within body of a match.

In other words, we can do match EXPR { #![inner_attr] ARM_1 ARM_2 ... } again.

I believe this unbreaks the only four crates that crater flagged as broken by PR #83312.

(I am putting this up so that the lang-team can check it out and decide whether it changes their mind about what to do regarding PR #83312.)

…hin body of a `match`.

In other words, we can do `match EXPR { #![inner_attr] ARM_1 ARM_2 ... }` again.

I believe this unbreaks the only four crates that crater flagged as broken by PR 83312.

(I am putting this up so that the lang-team can check it out and decide whether
it changes their mind about what to do regarding PR 83312.)
@rust-highfive
Copy link
Contributor

r? @matthewjasper

(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 May 11, 2021
@pnkfelix pnkfelix added the T-lang Relevant to the language team label May 11, 2021
@pnkfelix pnkfelix changed the title Re-add support for parsing (and pretty-printing) inner-attributes in match body [WIP] Re-add support for parsing (and pretty-printing) inner-attributes in match body May 11, 2021
@pnkfelix
Copy link
Contributor Author

nominating for T-lang meeting. If we're moving on this, I want it to happen on the 1.54 release (i.e. when PR #83312 landed). It would be nice to do it without having to beta-backport anything.

@pnkfelix pnkfelix changed the title [WIP] Re-add support for parsing (and pretty-printing) inner-attributes in match body Re-add support for parsing (and pretty-printing) inner-attributes in match body May 20, 2021
@joshtriplett
Copy link
Member

This seems reasonable, and should address the concerns in flight about having removed inner attributes previously.

Let's see if we have lang-team consensus on this. Note that this doesn't mean "we want to support attrs here", this is more "we acknowledge that there was breakage here and this fixes that breakage".

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Jun 1, 2021

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 1, 2021
@scottmcm
Copy link
Member

scottmcm commented Jun 1, 2021

@rfcbot reviewed

I'm not actually fond of inner attributes here, but re-enabling this to fix actual breakage sounds reasonable.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jun 1, 2021
@rfcbot
Copy link

rfcbot commented Jun 1, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@pnkfelix
Copy link
Contributor Author

If this is going to get into the 1.54 release without a beta-backport, it need to land before Friday 10 June.

it was assigning for review by @matthewjasper but I'm not actually sure that auto-assignment made perfect sense.

Nominating for (hopefully quick) discussion at today's compiler team meeting, to mainly see if I can pull in someone to do a review with quick turnaround.

@pnkfelix pnkfelix added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-lang Relevant to the language team labels Jun 10, 2021
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jun 11, 2021
@rfcbot
Copy link

rfcbot commented Jun 11, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@ehuss ehuss added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 12, 2021
@ehuss
Copy link
Contributor

ehuss commented Jun 12, 2021

Nominating for beta since this didn't merge in time.

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 17, 2021
@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 17, 2021
@Mark-Simulacrum
Copy link
Member

We should get this reviewed and merged. Perhaps @petrochenkov or @Aaron1011 could take a look? I'm not sure who is familiar with this area...

@nikomatsakis
Copy link
Contributor

@bors r+

This looks pretty straightforward.

@bors
Copy link
Collaborator

bors commented Jun 22, 2021

📌 Commit 8ce761d has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2021
@bors
Copy link
Collaborator

bors commented Jun 22, 2021

⌛ Testing commit 8ce761d with merge 6a758ea...

@bors
Copy link
Collaborator

bors commented Jun 22, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 6a758ea to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 22, 2021
@bors bors merged commit 6a758ea into rust-lang:master Jun 22, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 22, 2021
@cuviper cuviper mentioned this pull request Jul 2, 2021
@cuviper cuviper modified the milestones: 1.55.0, 1.54.0 Jul 2, 2021
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 2, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 2, 2021
[beta] backports

- rustfmt: load nested out-of-line mods correctly rust-lang#86424
- Re-add support for parsing (and pretty-printing) inner-attributes in match body rust-lang#85193
- Revert "List trait impls before methods from deref in the sidebar ..." rust-lang#86564
- Revert "Don't load all extern crates unconditionally" rust-lang#85749

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.