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

Merge ast::Mutability and mir::Mutability #67355

Merged
merged 3 commits into from
Dec 21, 2019
Merged

Merge ast::Mutability and mir::Mutability #67355

merged 3 commits into from
Dec 21, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Dec 16, 2019

@rust-highfive

This comment has been minimized.

Sign in to view
@Centril Centril added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Dec 16, 2019
@Centril
Copy link
Contributor Author

Centril commented Dec 16, 2019

N.B. This will break Clippy so merging should wait a bit.

@Centril Centril closed this Dec 18, 2019
@Centril Centril reopened this Dec 18, 2019
@Centril Centril added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 19, 2019
oli-obk
oli-obk reviewed Dec 20, 2019
View reviewed changes
src/librustdoc/clean/mod.rs
@@ -2101,8 +2101,8 @@ impl Clean<Item> for doctree::Constant<'_> {
impl Clean<Mutability> for hir::Mutability {
fn clean(&self, _: &DocContext<'_>) -> Mutability {
match self {
&hir::Mutability::Mutable => Mutable,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why has rustdoc its own mutability enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something rustdoc-wants-to-be-more-independent something something, cc @GuillaumeGomez

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do it for all hir types. If it can be removed, everyone will be happy though. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see why it couldn't be removed; should be quite easy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't look (and still haven't) so I don't know if it's possible or not. The idea behind clean rustdoc types is to provide a type with direct access to what rustdoc needs. If the rustdoc type doesn't provide anything more than the original, then it can (and should) be removed. Might be worth opening an issue about this.

oli-obk
oli-obk reviewed Dec 20, 2019
View reviewed changes
src/libsyntax/print/pprust.rs
@@ -1083,7 +1083,7 @@ impl<'a> State<'a> {
}
ast::ForeignItemKind::Static(ref t, m) => {
self.head(visibility_qualified(&item.vis, "static"));
if m == ast::Mutability::Mutable {
if m == ast::Mutability::Mut {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these comparisons make me wonder if we should have is_mut() and is_immut() helper methods

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest just nixing PartialEq by using the beautiful if let instead.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 20, 2019

@bors r+

@rust-lang/clippy (trivial) but a lot of breakage incoming

@bors
Copy link
Contributor

bors commented Dec 20, 2019

📌 Commit 9fa6f34d3ec540efd6e7465c0af54d47911c809e has been approved by oli-obk

@bors
Copy link
Contributor

bors commented Dec 20, 2019

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

@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 20, 2019
@bors

This comment has been minimized.

Sign in to view
@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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 20, 2019
@Centril
Copy link
Contributor Author

Centril commented Dec 20, 2019

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Dec 20, 2019

📌 Commit 6d7c6d7 has been approved by oli-obk

@bors
Copy link
Contributor

bors commented Dec 20, 2019

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 20, 2019
@GuillaumeGomez GuillaumeGomez mentioned this pull request Dec 21, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Dec 21, 2019
@Centril
Rollup merge of rust-lang#67355 - Centril:merge-mut, r=oli-obk
c0bf3af 
Merge `ast::Mutability` and `mir::Mutability`

r? @oli-obk
@Centril Centril mentioned this pull request Dec 21, 2019
Merged
bors added a commit that referenced this pull request Dec 21, 2019
@bors
Auto merge of #67485 - Centril:rollup-gt0opvr, r=Centril
0a440b1 
Rollup of 7 pull requests

Successful merges:

 - #67059 (Fix too restrictive checks on Drop impls)
 - #67355 (Merge `ast::Mutability` and `mir::Mutability`)
 - #67393 (Enable opting out of specific default LLVM arguments.)
 - #67422 (Cleanup err codes)
 - #67462 (Make ptr::slice_from_raw_parts a const fn available under a feature flag)
 - #67467 (Test slice patterns more)
 - #67478 (Fix src/libcore/str/mod.rs doc comments)

Failed merges:

r? @ghost
@Centril Centril mentioned this pull request Dec 21, 2019
Merged
@bors bors merged commit 6d7c6d7 into rust-lang:master Dec 21, 2019
@Centril Centril deleted the merge-mut branch December 21, 2019 17:36
@tesuji tesuji mentioned this pull request Dec 21, 2019
Merged
bors added a commit to rust-lang/rust-clippy that referenced this pull request Dec 21, 2019
@bors
Auto merge of #4932 - lzutao:rustup-67355, r=matthiaskrgr
e3426bf 
rustup "Merge `ast::Mutability` and `mir::Mutability`"

cc rust-lang/rust#67355
changelog: none
bors added a commit to rust-lang/rust-clippy that referenced this pull request Dec 21, 2019
@bors
Auto merge of #4932 - lzutao:rustup-67355, r=matthiaskrgr
cfb3320 
rustup "Merge `ast::Mutability` and `mir::Mutability`"

cc rust-lang/rust#67355
changelog: none
calebcartwright added a commit to calebcartwright/rustfmt that referenced this pull request Jan 16, 2020
@calebcartwright
refactor: update ast::Mutabiltiy variants
f87fe48 
`Mutability` variants were renamed to `Not` and `Mut`
rust-lang/rust#67355
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez left review comments

@oli-obk oli-obk oli-obk left review comments

Assignees

@oli-obk oli-obk

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.
5 participants
@Centril @rust-highfive @oli-obk @bors @GuillaumeGomez