refactor: make clippy almost silent, some optimizations #916
Conversation
yusdacra
force-pushed
the
refactor/clippy-fixes
branch
from
9c3e001 to
c8f1b19
Compare
yusdacra
changed the title
yusdacra
force-pushed
the
refactor/clippy-fixes
branch
from
c8f1b19 to
8dc5e86
Compare
yusdacra
force-pushed
the
refactor/clippy-fixes
branch
4 times, most recently
from
d10e3f6 to
a94de1d
Compare
yusdacra
force-pushed
the
refactor/clippy-fixes
branch
from
a94de1d to
9be7a36
Compare
| f32::from(text_size + self.padding.vertical()) | ||
| * self.options.len() as f32, | ||
| f32::from( | ||
| self.text_size.unwrap_or(renderer.default_size()) |
There was a problem hiding this comment.
self.text_size.unwrap_or_else(|| renderer.default_size()) better.
A little memory optimization here and not have to make an exclusion for the linter.
I would even make a separate structure with Default and Deref into u16. Then you could do .unwrap_or_default().
Especially since such an expression is very often used in the code.
There was a problem hiding this comment.
This also returns a u16 which is stack allocated. Since it will be allocated in the stack we don't need lazy eval, and it's very cheap anyways (ideally default_size would be marked as const, but trait const fn's aren't finished yet), so we don't need the indirection.
There was a problem hiding this comment.
On second thought; Rust should figure out and just inline the value, bypassing the lazyness.
| .filter(|(_, word)| !word.trim_start().is_empty()) | ||
| .next() | ||
| #[allow(clippy::or_fun_call)] | ||
| // self.len() is very cheap, we don't need lazy eval |
There was a problem hiding this comment.
But why allocate memory every time it is not used?
There was a problem hiding this comment.
self.len() doesn't allocate any memory; it returns a usize which is stored on the stack. It will be stored on the stack even if we put it in a closure; so the closure is needless indirection.
There was a problem hiding this comment.
Same goes for here; Rust should inline the value.
hecrj
force-pushed
the
master
branch
2 times, most recently
from
e6ab610 to
8b0f2e6
Compare
|
Superseded by #1379. |
I was annoyed by the many clippy warnings so I tried to make it shut up as much as possible.
This PR also reduces allocations in
text_input. It adds one new dependency, smol_str.Commit 9be7a36 changes some public API (especially
Toggler) to make them takeInto<SmolStr>instead ofInto<String>. This causes some minimal breakages (easily fixable, see tour example changes + i think it looks nicer), but if it's not okay I can leave it out.