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.

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

Collapse new_without_default and new_without_default_derive into a single lint #3525

Closed
dtolnay opened this issue Dec 10, 2018 · 0 comments
Closed
Labels
good-first-issue These issues are a good way to get started with Clippy

Comments

@dtolnay
Copy link
Member

dtolnay commented Dec 10, 2018

In code that wants to suppress new_without_default, but which uses a mix of types only some of which could derive Default, it is annoying to have to suppress both of these lint names. I consider them effectively the same lint.

I would like to propose deprecating new_without_default_derive and having new_without_default lint all the cases that used to be linted by new_without_default_derive. The cases that can derive Default should retain the help message that mentions deriving Default.

The way to think about this is: if a codebase wants to have new functions without necessarily having Default impls, then it won't matter to them whether or not Default can be derived. They would just suppress both lints. Every additional lint that they find needs to be suppressed detracts from their impression of Clippy.

#![allow(clippy::new_without_default, clippy::new_without_default_derive)]

pub struct S1;

pub struct S2(*const u8);

impl S1 {
    pub fn new() -> Self {
        S1
    }
}

impl S2 {
    pub fn new() -> Self {
        S2(0 as _)
    }
}

clippy 0.0.212 (29bf75c 2018-12-05)

@flip1995 flip1995 added the good-first-issue These issues are a good way to get started with Clippy label Dec 12, 2018
bors added a commit that referenced this issue Dec 29, 2018
Merge new_without_default_derive into new_without_default

Closes #3525, deprecating new_without_default_derive and moving both lints into new_without_default.
baumanj added a commit to habitat-sh/habitat that referenced this issue Mar 6, 2019
See rust-lang/rust-clippy#3525

Signed-off-by: Jon Bauman <5906042+baumanj@users.noreply.github.com>
baumanj added a commit to habitat-sh/habitat that referenced this issue Mar 12, 2019
See rust-lang/rust-clippy#3525

Signed-off-by: Jon Bauman <5906042+baumanj@users.noreply.github.com>
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue May 24, 2024
…eferences, r=RalfJung

Make file descriptors into refcount references

fixes rust-lang#3525

Remove `fn dup` in `trait FileDescription`, define `struct FileDescriptor(Rc<RefCell<dyn FileDescription>>)`, and use `BTreeMap<i32, FileDescriptor>` in `FdTable`.

---

There are some refactors similar to the following form:
```rust
{  // origin:
    if let Some(file_descriptor) = this.machine.fds.get_mut(fd) {
        // write file_descriptor
        this.try_unwrap_io_result(result)
    } else {
        this.fd_not_found()
    }
}
{  // now:
    let Some(mut file_descriptor) = this.machine.fds.get_mut(fd) else {
        return this.fd_not_found();
    };
    // write file_descriptor
    drop(file_descriptor);
    this.try_unwrap_io_result(result)
}
```
The origin form can't compile because as using `RefCell` to get interior mutability, `fn get_mut` return `Option<std::cell::RefMut<'_, dyn FileDescription>>` instead of `Option<&mut dyn FileDescription>` now, and the `deref_mut` on `file_descriptor: RefMut` will cause borrow `this` as mutable more than once at a time.
So this form of refactors and manual drops are are implemented to avoid borrowing `this` at the same time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

No branches or pull requests

2 participants