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.

Sign up for GitHub

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

Jump to bottom

suggest ; on expr mac!() which is good as stmt mac!() #70434

Merged
merged 1 commit into from
Mar 28, 2020
Merged

suggest ; on expr mac!() which is good as stmt mac!() #70434

merged 1 commit into from
Mar 28, 2020

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Mar 26, 2020

Fixes #34421 by implementing @jseyfried's suggestion in #34421 (comment).

r? @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 26, 2020
@petrochenkov
Copy link
Contributor

I'm ok with this if perf is unaffected, but I don't care to much about fixing this because the example should compile successfully in the token-based expansion model.

r? @estebank

@Centril
Copy link
Contributor Author

Centril commented Mar 26, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Mar 26, 2020

⌛ Trying commit cbfa75e139d9c434fd31b2e59ac118e0b9040b46 with merge d12c42b713ed6844182c19063db4f6da6dbc2dc0...

@bors
Copy link
Contributor

bors commented Mar 26, 2020

💔 Test failed - checks-azure

@bors bors 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 Mar 26, 2020
@Centril
Copy link
Contributor Author

Centril commented Mar 26, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Mar 26, 2020

⌛ Trying commit cbfa75e139d9c434fd31b2e59ac118e0b9040b46 with merge a04f461d8ae91c3ec42f8743523878d8e5174b23...

@bors
Copy link
Contributor

bors commented Mar 26, 2020

💔 Test failed - checks-azure

@Mark-Simulacrum
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Mar 26, 2020

⌛ Trying commit cbfa75e139d9c434fd31b2e59ac118e0b9040b46 with merge 73876af7e044d371e84684916fa2df78bb8d7a00...

@bors
Copy link
Contributor

bors commented Mar 26, 2020

☀️ Try build successful - checks-azure
Build commit: 73876af7e044d371e84684916fa2df78bb8d7a00 (73876af7e044d371e84684916fa2df78bb8d7a00)

@rust-timer
Copy link
Collaborator

Queued 73876af7e044d371e84684916fa2df78bb8d7a00 with parent 2fbb075, future comparison URL.

@Centril
Copy link
Contributor Author

Centril commented Mar 27, 2020

Perfbot suggests this was a perf win but I suspect that's bogus. At least it's not a regression, so I'll make the other requested changes. :)

@Centril
Copy link
Contributor Author

Centril commented Mar 27, 2020

@estebank I've addressed your comments. :)

@Centril Centril removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 27, 2020
@Centril Centril added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2020
@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 27, 2020

📌 Commit 6c643a0 has been approved by estebank

@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 Mar 27, 2020
Centril added a commit to Centril/rust that referenced this pull request Mar 27, 2020
@Centril
Rollup merge of rust-lang#70434 - Centril:fix-34421, r=estebank
d79ff74 
suggest `;` on expr `mac!()` which is good as stmt `mac!()`

Fixes rust-lang#34421 by implementing @jseyfried's suggestion in rust-lang#34421 (comment).

r? @petrochenkov
@Centril Centril mentioned this pull request Mar 27, 2020
Closed
@Centril Centril mentioned this pull request Mar 27, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 28, 2020
@bors
Auto merge of rust-lang#70483 - Centril:rollup-slli4yf, r=Centril
2acf32d 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#70345 (Remove `no_integrated_as` mode.)
 - rust-lang#70434 (suggest `;` on expr `mac!()` which is good as stmt `mac!()`)
 - rust-lang#70457 (non-exhastive diagnostic: add note re. scrutinee type)
 - rust-lang#70478 (Refactor type_of for constants)
 - rust-lang#70480 (clarify hir_id <-> node_id method names)

Failed merges:

r? @ghost
@bors bors merged commit cfe1e33 into rust-lang:master Mar 28, 2020
@Centril Centril deleted the fix-34421 branch March 28, 2020 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank left review comments

Assignees

@estebank estebank

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

error message for macro in wrong context is missing context
7 participants
@Centril @petrochenkov @rust-timer @bors @Mark-Simulacrum @estebank @rust-highfive