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

rustc_builtin_macros: make asm mod public for rustfmt #92147

Merged
merged 1 commit into from
Dec 28, 2021
Merged

rustc_builtin_macros: make asm mod public for rustfmt #92147

merged 1 commit into from
Dec 28, 2021
+3 −3

Conversation

calebcartwright
Copy link
Member

Follow up to #92016, as I'd completely missed that the mod we needed was internal

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 21, 2021
@rust-highfive
Copy link
Collaborator

r? @cjgillot

(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 Dec 21, 2021
@jyn514
Copy link
Member

jyn514 commented Dec 21, 2021

Hmm, I think it would be better to re-export the function in the root rather than making the whole module public.

@jyn514 jyn514 added the A-rustfmt Area: Rustfmt label Dec 21, 2021
@calebcartwright
Copy link
Member Author

Hmm, I think it would be better to re-export the function in the root rather than making the whole module public.

Well, we need the struct too. There's only 4 currently public items in that mod, so I suppose I can either re-export the 2 we need and keep the mod private, or modify the visibility on the other 2 to keep them in-crate, if there's concerns about expand_asm or expand_global_asm being used elsewhere in an undesirable manner (or similarly for any hypothetical future pub items added to asm)

@calebcartwright calebcartwright force-pushed the publicize-builtin_macro-asm branch from 2133d12 to 63c2ede Compare December 21, 2021 17:52
@calebcartwright
Copy link
Member Author

I've opted for the latter, but don't mind changing it if you (or anyone) feels strongly that it'd be better to do the former. Let me know

@cjgillot
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 26, 2021

📌 Commit 63c2ede has been approved by cjgillot

@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 Dec 26, 2021
@bors
Copy link
Contributor

bors commented Dec 27, 2021

⌛ Testing commit 63c2ede with merge 6d68b965a62b8b3c30a546673b867578a040c0f5...

@bors
Copy link
Contributor

bors commented Dec 27, 2021

💔 Test failed - checks-actions

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

Not really sure what to make of the CI failure, seems potentially spurious so going to retry

image

@bors retry

@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 Dec 27, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@matthiaskrgr matthiaskrgr mentioned this pull request Dec 27, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 27, 2021
@bors
Auto merge of rust-lang#92329 - matthiaskrgr:rollup-l3b4fl1, r=matthi…
4ee34f3 
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90586 (Relax priv-in-pub lint on generic bounds and where clauses of trait impls.)
 - rust-lang#92112 (Fix the error of checking `base_expr` twice in type_changing_struct_update)
 - rust-lang#92147 (rustc_builtin_macros: make asm mod public for rustfmt)
 - rust-lang#92161 (resolve: Minor miscellaneous cleanups from rust-lang#89059)
 - rust-lang#92264 (Remove `maybe_uninit_extra` feature from Vec docs)
 - rust-lang#92303 (Add test cases for issue rust-lang#26186)
 - rust-lang#92307 (Fix minor typos)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a7c79c0 into rust-lang:master Dec 28, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 28, 2021
@calebcartwright calebcartwright deleted the publicize-builtin_macro-asm branch December 28, 2021 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cjgillot cjgillot

Labels
A-rustfmt Area: Rustfmt 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
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.
7 participants
@calebcartwright @rust-highfive @jyn514 @cjgillot @bors @rust-log-analyzer @rustbot