Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Trie fixes #5408

Closed
wants to merge 3 commits into from
Closed

Trie fixes #5408

wants to merge 3 commits into from

Conversation

thestinger
Copy link
Contributor

The chunk fix is cherry picked from @graydon's gc branch.

bors added a commit that referenced this pull request Mar 16, 2013
@bors
auto merge of #5408 : thestinger/rust/trie, r=pcwalton
ebba8b4 
The chunk fix is cherry picked from @graydon's `gc` branch.
@bors bors closed this Mar 16, 2013
oli-obk pushed a commit to oli-obk/rust that referenced this pull request May 2, 2020
@flip1995
Rollup merge of rust-lang#5408 - dtolnay:matchbool, r=flip1995
e1d13c3 
Downgrade match_bool to pedantic

I don't quite buy the justification in https://rust-lang.github.io/rust-clippy/. The justification is:

> It makes the code less readable.

In the Rust codebases I've worked in, I have found people were comfortable using `match bool` (selectively) to make code more readable. For example, initializing struct fields is a place where the indentation of `match` can work better than the indentation of `if`:

```rust
let _ = Struct {
    v: {
        ...
    },
    w: match doing_w {
        true => ...,
        false => ...,
    },
    x: Nested {
        c: ...,
        b: ...,
        a: ...,
    },
    y: if doing_y {
        ...
    } else { // :(
        ...
    },
    z: ...,
};
```

Or sometimes people prefer something a bit less pithy than `if` when the meaning of the bool doesn't read off clearly from the condition:

```rust
if set.insert(...) {
    ... // ???
} else {
    ...
}

match set.insert(...) {
    // set.insert returns false if already present
    false => ...,
    true => ...,
}
```

Or `match` can be a better fit when the bool is playing the role more of a value than a branch condition:

```rust
impl ErrorCodes {
    pub fn from(b: bool) -> Self {
        match b {
            true => ErrorCodes::Yes,
            false => ErrorCodes::No,
        }
    }
}
```

And then there's plain old it's-1-line-shorter, which means we get 25% more content on a screen when stacking a sequence of conditions:

```rust
let old_noun = match old_binding.is_import() {
    true => "import",
    false => "definition",
};
let new_participle = match new_binding.is_import() {
    true => "imported",
    false => "defined",
};
```

Bottom line is I think this lint fits the bill better as a pedantic lint; I don't think linting on this by default is justified.

changelog: Remove match_bool from default set of enabled lints
oli-obk pushed a commit to oli-obk/rust that referenced this pull request May 2, 2020
@bors
Auto merge of rust-lang#5527 - flip1995:rollup-pr2htfd, r=flip1995
77c23b7 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#5408 (Downgrade match_bool to pedantic)
 - rust-lang#5505 (Avoid running cargo+internal lints when not enabled)
 - rust-lang#5516 (Add a note to the beta sections of release.md)
 - rust-lang#5517 (Deploy time travel)
 - rust-lang#5523 (Add lifetime test case for `new_ret_no_self`)

Failed merges:

r? @ghost

changelog: rollup
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thestinger @bors @graydon