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

refactor(lib): apply small refactoring #677

Merged
merged 19 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c54d8e0
refactor(lib): remove redundant field name
tottoto Feb 16, 2024
18fb2bc
refactor(lib): refactor conditional expression
tottoto Feb 16, 2024
fae9e0d
refactor(lib): remove redundant reference
tottoto Feb 16, 2024
4a27676
refactor(uri): use more idiomatic api
tottoto Feb 16, 2024
e981e58
refactor(header): remove redundant type conversion
tottoto Feb 16, 2024
67bfb42
refactor(header): remove redundant slicing
tottoto Feb 16, 2024
eb42b0e
refactor(uri): replace single string with character
tottoto Feb 16, 2024
91cc6e9
refactor(lib): refactor redundant closure
tottoto Feb 16, 2024
acf6ee4
refactor(lib): remove unused lifetime parameter
tottoto Feb 16, 2024
a64956a
refactor(header): remove manual deref
tottoto Feb 16, 2024
5663e23
refactor(header): resolve non-canonical PartialOrd implementation
tottoto Feb 16, 2024
74e930d
chore(header): make HeaderValue Hash implementation consistent with Eq
tottoto Feb 16, 2024
741f0df
refactor(lib): use to_owned to clarify the purpose
tottoto Feb 16, 2024
4e47afb
refactor(header): ownership is not needed to iterate
tottoto Feb 16, 2024
5cace4a
refactor(status): remove redundant static lifetime
tottoto Feb 16, 2024
94144fc
refactor(header): add comment and lint allowing to panic in const con…
tottoto Feb 16, 2024
1d8451b
doc(header): add panics and safety section to document
tottoto Feb 16, 2024
cd9b266
refactor(header): rename method to follow naming convention
tottoto Feb 16, 2024
f450ada
chore(header): allow clippy::should_implement_trait rule for HeaderVa…
tottoto Feb 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/byte_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl ByteStr {
}
}
// Invariant: assumed by the safety requirements of this function.
ByteStr { bytes: bytes }
ByteStr { bytes }
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl Extensions {
/// ```
pub fn insert<T: Clone + Send + Sync + 'static>(&mut self, val: T) -> Option<T> {
self.map
.get_or_insert_with(|| Box::new(HashMap::default()))
.get_or_insert_with(Box::default)
.insert(TypeId::of::<T>(), Box::new(val))
.and_then(|boxed| boxed.into_any().downcast().ok().map(|boxed| *boxed))
}
Expand All @@ -82,7 +82,7 @@ impl Extensions {
self.map
.as_ref()
.and_then(|map| map.get(&TypeId::of::<T>()))
.and_then(|boxed| (&**boxed).as_any().downcast_ref())
.and_then(|boxed| (**boxed).as_any().downcast_ref())
}

/// Get a mutable reference to a type previously inserted on this `Extensions`.
Expand All @@ -101,7 +101,7 @@ impl Extensions {
self.map
.as_mut()
.and_then(|map| map.get_mut(&TypeId::of::<T>()))
.and_then(|boxed| (&mut **boxed).as_any_mut().downcast_mut())
.and_then(|boxed| (**boxed).as_any_mut().downcast_mut())
}

/// Get a mutable reference to a type, inserting `value` if not already present on this
Expand Down Expand Up @@ -138,10 +138,10 @@ impl Extensions {
) -> &mut T {
let out = self
.map
.get_or_insert_with(|| Box::new(HashMap::default()))
.get_or_insert_with(Box::default)
.entry(TypeId::of::<T>())
.or_insert_with(|| Box::new(f()));
(&mut **out).as_any_mut().downcast_mut().unwrap()
(**out).as_any_mut().downcast_mut().unwrap()
}

/// Get a mutable reference to a type, inserting the type's default value if not already present
Expand Down
95 changes: 43 additions & 52 deletions src/header/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ impl<T> HeaderMap<T> {
assert!(cap <= MAX_SIZE, "header map reserve over max capacity");
assert!(cap != 0, "header map reserve overflowed");

if self.entries.len() == 0 {
if self.entries.is_empty() {
self.mask = cap as Size - 1;
self.indices = vec![Pos::none(); cap].into_boxed_slice();
self.entries = Vec::with_capacity(usable_capacity(cap));
Expand Down Expand Up @@ -1092,22 +1092,22 @@ impl<T> HeaderMap<T> {
danger,
Entry::Vacant(VacantEntry {
map: self,
hash: hash,
hash,
key: key.into(),
probe: probe,
danger: danger,
probe,
danger,
}),
Entry::Occupied(OccupiedEntry {
map: self,
index: pos,
probe: probe,
probe,
}),
Entry::Vacant(VacantEntry {
map: self,
hash: hash,
hash,
key: key.into(),
probe: probe,
danger: danger,
probe,
danger,
})
)
}
Expand Down Expand Up @@ -1209,7 +1209,7 @@ impl<T> HeaderMap<T> {

ValueDrain {
first: Some(old),
next: next,
next,
lt: PhantomData,
}
}
Expand Down Expand Up @@ -1333,7 +1333,7 @@ impl<T> HeaderMap<T> {

if danger || num_displaced >= DISPLACEMENT_THRESHOLD {
// Increase danger level
self.danger.to_yellow();
self.danger.set_yellow();
}

index
Expand Down Expand Up @@ -1415,7 +1415,7 @@ impl<T> HeaderMap<T> {

// backward shift deletion in self.indices
// after probe, shift all non-ideally placed indices backward
if self.entries.len() > 0 {
if !self.entries.is_empty() {
let mut last_probe = probe;
let mut probe = probe + 1;

Expand Down Expand Up @@ -1462,9 +1462,9 @@ impl<T> HeaderMap<T> {
assert!(self.entries.len() < MAX_SIZE, "header map at capacity");

self.entries.push(Bucket {
hash: hash,
key: key,
value: value,
hash,
key,
value,
links: None,
});
}
Expand Down Expand Up @@ -1524,15 +1524,15 @@ impl<T> HeaderMap<T> {

if load_factor >= LOAD_FACTOR_THRESHOLD {
// Transition back to green danger level
self.danger.to_green();
self.danger.set_green();

// Double the capacity
let new_cap = self.indices.len() * 2;

// Grow the capacity
self.grow(new_cap);
} else {
self.danger.to_red();
self.danger.set_red();

// Rebuild hash table
for index in self.indices.iter_mut() {
Expand Down Expand Up @@ -1855,7 +1855,7 @@ where
type Error = Error;

fn try_from(c: &'a HashMap<K, V>) -> Result<Self, Self::Error> {
c.into_iter()
c.iter()
.map(|(k, v)| -> crate::Result<(HeaderName, T)> {
let name = TryFrom::try_from(k).map_err(Into::into)?;
let value = TryFrom::try_from(v).map_err(Into::into)?;
Expand Down Expand Up @@ -1992,7 +1992,7 @@ impl<T> Default for HeaderMap<T> {
}
}

impl<'a, K, T> ops::Index<K> for HeaderMap<T>
impl<K, T> ops::Index<K> for HeaderMap<T>
where
K: AsHeaderName,
{
Expand Down Expand Up @@ -2042,7 +2042,7 @@ fn append_value<T>(
Some(links) => {
let idx = extra.len();
extra.push(ExtraValue {
value: value,
value,
prev: Link::Extra(links.tail),
next: Link::Entry(entry_idx),
});
Expand All @@ -2054,7 +2054,7 @@ fn append_value<T>(
None => {
let idx = extra.len();
extra.push(ExtraValue {
value: value,
value,
prev: Link::Entry(entry_idx),
next: Link::Entry(entry_idx),
});
Expand Down Expand Up @@ -2461,9 +2461,9 @@ impl<'a, T> VacantEntry<'a, T> {
/// ```
pub fn insert(self, value: T) -> &'a mut T {
// Ensure that there is space in the map
let index =
self.map
.insert_phase_two(self.key, value.into(), self.hash, self.probe, self.danger);
let index = self
.map
.insert_phase_two(self.key, value, self.hash, self.probe, self.danger);

&mut self.map.entries[index].value
}
Expand All @@ -2488,13 +2488,13 @@ impl<'a, T> VacantEntry<'a, T> {
/// ```
pub fn insert_entry(self, value: T) -> OccupiedEntry<'a, T> {
// Ensure that there is space in the map
let index =
self.map
.insert_phase_two(self.key, value.into(), self.hash, self.probe, self.danger);
let index = self
.map
.insert_phase_two(self.key, value, self.hash, self.probe, self.danger);

OccupiedEntry {
map: self.map,
index: index,
index,
probe: self.probe,
}
}
Expand Down Expand Up @@ -2900,7 +2900,7 @@ impl<'a, T> OccupiedEntry<'a, T> {
/// assert_eq!("earth", map["host"]);
/// ```
pub fn insert(&mut self, value: T) -> T {
self.map.insert_occupied(self.index, value.into())
self.map.insert_occupied(self.index, value)
}

/// Sets the value of the entry.
Expand All @@ -2926,7 +2926,7 @@ impl<'a, T> OccupiedEntry<'a, T> {
/// assert_eq!("earth", map["host"]);
/// ```
pub fn insert_mult(&mut self, value: T) -> ValueDrain<'_, T> {
self.map.insert_occupied_mult(self.index, value.into())
self.map.insert_occupied_mult(self.index, value)
}

/// Insert the value into the entry.
Expand All @@ -2953,7 +2953,7 @@ impl<'a, T> OccupiedEntry<'a, T> {
pub fn append(&mut self, value: T) {
let idx = self.index;
let entry = &mut self.map.entries[idx];
append_value(idx, entry, &mut self.map.extra_values, value.into());
append_value(idx, entry, &mut self.map.extra_values, value);
}

/// Remove the entry from the map.
Expand Down Expand Up @@ -3131,12 +3131,12 @@ impl<'a, T> Iterator for ValueDrain<'a, T> {
// Exactly 1
(&Some(_), &None) => (1, Some(1)),
// 1 + extras
(&Some(_), &Some(ref extras)) => {
(&Some(_), Some(extras)) => {
let (l, u) = extras.size_hint();
(l + 1, u.map(|u| u + 1))
}
// Extras only
(&None, &Some(ref extras)) => extras.size_hint(),
(&None, Some(extras)) => extras.size_hint(),
// No more
(&None, &None) => (0, Some(0)),
}
Expand All @@ -3147,7 +3147,7 @@ impl<'a, T> FusedIterator for ValueDrain<'a, T> {}

impl<'a, T> Drop for ValueDrain<'a, T> {
fn drop(&mut self) {
while let Some(_) = self.next() {}
for _ in self.by_ref() {}
}
}

Expand Down Expand Up @@ -3186,7 +3186,7 @@ impl Pos {
debug_assert!(index < MAX_SIZE);
Pos {
index: index as Size,
hash: hash,
hash,
}
}

Expand Down Expand Up @@ -3220,34 +3220,25 @@ impl Pos {

impl Danger {
fn is_red(&self) -> bool {
match *self {
Danger::Red(_) => true,
_ => false,
}
matches!(*self, Danger::Red(_))
}

fn to_red(&mut self) {
fn set_red(&mut self) {
debug_assert!(self.is_yellow());
*self = Danger::Red(RandomState::new());
}

fn is_yellow(&self) -> bool {
match *self {
Danger::Yellow => true,
_ => false,
}
matches!(*self, Danger::Yellow)
}

fn to_yellow(&mut self) {
match *self {
Danger::Green => {
*self = Danger::Yellow;
}
_ => {}
fn set_yellow(&mut self) {
if let Danger::Green = *self {
*self = Danger::Yellow;
}
}

fn to_green(&mut self) {
fn set_green(&mut self) {
debug_assert!(self.is_yellow());
*self = Danger::Green;
}
Expand Down Expand Up @@ -3456,7 +3447,7 @@ mod as_header_name {
}

fn as_str(&self) -> &str {
<HeaderName>::as_str(*self)
<HeaderName>::as_str(self)
}
}

Expand Down Expand Up @@ -3510,7 +3501,7 @@ mod as_header_name {
}

fn as_str(&self) -> &str {
*self
self
}
}

Expand Down
17 changes: 10 additions & 7 deletions src/header/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ impl HeaderName {
};
}

if name_bytes.len() == 0 || name_bytes.len() > super::MAX_HEADER_NAME_LEN || {
if name_bytes.is_empty() || name_bytes.len() > super::MAX_HEADER_NAME_LEN || {
let mut i = 0;
loop {
if i >= name_bytes.len() {
Expand All @@ -1269,6 +1269,12 @@ impl HeaderName {
i += 1;
}
} {
// TODO: When msrv is bumped to larger than 1.57, this should be
// replaced with `panic!` macro.
// https://blog.rust-lang.org/2021/12/02/Rust-1.57.0.html#panic-in-const-contexts
//
// See the panics section of this method's document for details.
#[allow(clippy::no_effect)]
([] as [u8; 0])[0]; // Invalid header name
}

Expand All @@ -1284,7 +1290,7 @@ impl HeaderName {
pub fn as_str(&self) -> &str {
match self.inner {
Repr::Standard(v) => v.as_str(),
Repr::Custom(ref v) => &*v.0,
Repr::Custom(ref v) => &v.0,
}
}

Expand Down Expand Up @@ -1516,10 +1522,7 @@ impl<'a> HdrName<'a> {
fn custom(buf: &'a [u8], lower: bool) -> HdrName<'a> {
HdrName {
// Invariant (on MaybeLower): follows from the precondition
inner: Repr::Custom(MaybeLower {
buf: buf,
lower: lower,
}),
inner: Repr::Custom(MaybeLower { buf, lower }),
}
}

Expand Down Expand Up @@ -1554,7 +1557,7 @@ impl<'a> From<HdrName<'a>> for HeaderName {
},
Repr::Custom(maybe_lower) => {
if maybe_lower.lower {
let buf = Bytes::copy_from_slice(&maybe_lower.buf[..]);
let buf = Bytes::copy_from_slice(maybe_lower.buf);
// Safety: the invariant on MaybeLower ensures buf is valid UTF-8.
let byte_str = unsafe { ByteStr::from_utf8_unchecked(buf) };

Expand Down
Loading