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

fix a whole bunch of annoying clippy lints #7

Merged
merged 10 commits into from
Jul 8, 2022
Merged

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Jul 8, 2022

I have clippy enabled by default in VS Code, and it currently complains
about a bunch of stuff...most of which is just kind of annoying
pedantry. This branch fixes all those lints; in some cases, this
actually does make the code somewhat nicer (IMO), but mostly it makes
the warnings in my VS Code "problems" window go away.

I've applied the fix for each lint in a separate commit. If there are
any of these changes we would prefer not to apply, the individual commit
can be backed out and replaced with an #[allow(...)] attribute or
whatever, as appropriate.

Hope this isn't too annoying of a change...I can also just turn off clippy for
this repo, or learn to live with the warnings.

hawkw added 10 commits July 8, 2022 09:45
Clippy lints on using `ptr.offset(usize_val as isize)` instead of
`ptr.add(usize_val)`. This fixes that lint (and, I think, makes the code
a bit nicer).

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
As suggested by clippy.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Clippy complains about this: because `pos` and `mask` are already
`usize`s, the `usize::from` does nothing.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Clippy complains about this and suggests it be rewritten to match on
`dif.cmp(0)`: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain

However, there are potential performance issues with using `cmp`, as it
may not always be inlined (see rust-lang/rust-clippy#5354). Therefore,
I rewrote these to use a match without `cmp`, which silences the lint.
I'm...not sure if the resultant code is clearer than the previous code,
so, we could alternatively just add an `#[allow(...)]` attribute for
this lint. @jamesmunns, what do you think?

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Clippy doesn't like that

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
There's a bunch of these and I didn't want to write a # Safety section
for all of this code, especially given that I didn't write it...so I
just turned off the lints.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Clippy doesn't like functions returning `Result<..., ()>`. This will
also probably make debugging any potential kernel initialization errors
a bit less painful.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw requested a review from jamesmunns July 8, 2022 17:07
@jamesmunns
Copy link
Contributor

Love it!

@jamesmunns jamesmunns merged commit 916909d into main Jul 8, 2022
@jamesmunns jamesmunns deleted the eliza/clippy branch July 17, 2022 00:01
jamesmunns pushed a commit that referenced this pull request Jun 4, 2023
This branch updates the `cordyceps` and `maitake` dependencies to
hawkw/mycelium@5e46e35.
jamesmunns added a commit that referenced this pull request Jun 4, 2023
These are the "non-`disk`" changes made for #2, without the actual "add the disk" support.

I'm opening this as a PR to hopefully allow @hawkw to merge it into her branches while I'm still working on disk branch.

Uncontroversial changes:

* Makes EntryKind non-exhaustive - could be used for robustness checking
* Adds a single method to allocation a dictionary entry, used to remove some copy/paste code
* adds `b@` and `b!` operators for byte-aligned (instead of cell-aligned) load/stores

Potentially controversial changes:

* Adds a `ptr_data: isize` union variant to the cell type, to be used when mixing "i32" data from the stack with "ptr" data from the stack
* ONLY `add` and `sub` actually use the `ptr_data` variant

At some point, we might want to pass and declare which operators ARE "pointer safe", and which ones aren't, or add specific ptr-safe operators. That being said, I'm not sure whether any operations OTHER than add or sub on a pointer are likely to be meaningful.

* Unrelated changes made during `disk` changes
* Update src/leakbox.rs
* Rename for consistency

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@jamesmunns jamesmunns added the kind: enhancement New feature or request label Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants