Skip to content

Commit 1ca63b1

Browse files
committed
Use nonmax::NonMax* types in Options wherever possible (#2681)
This swaps fields that store `Option<Int>` for `Option<NonMaxInt>` where the maximum value would be ludicrous. Since `nonmax` uses `NonZero` internally, this gives us niche optimisations, so model sizes can drop some more. I have had to include a workaround for [#17] in `optional_string` by making my own `TryFrom<u64>`, so that should be removable once that issue is fixed. [#17]: LPGhatguy/nonmax#17
1 parent 9f7d223 commit 1ca63b1

File tree

15 files changed

+125
-62
lines changed

15 files changed

+125
-62
lines changed

Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ arrayvec = { version = "0.7.4", features = ["serde"] }
3333
serde_cow = { version = "0.1.0" }
3434
small-fixed-array = { git = "https://github.com/GnomedDev/small-fixed-array", features = ["serde", "log_using_tracing"] }
3535
bool_to_bitflags = { git = "https://github.com/GnomedDev/bool-to-bitflags", version = "0.1.0" }
36+
nonmax = { version = "0.5.5", features = ["serde"] }
3637
# Optional dependencies
3738
fxhash = { version = "0.2.1", optional = true }
3839
simd-json = { version = "0.13.4", optional = true }
@@ -51,7 +52,7 @@ mime_guess = { version = "2.0.4", optional = true }
5152
dashmap = { version = "5.5.3", features = ["serde"], optional = true }
5253
parking_lot = { version = "0.12.1", optional = true }
5354
ed25519-dalek = { version = "2.0.0", optional = true }
54-
typesize = { version = "0.1.4", optional = true, features = ["url", "time", "serde_json", "secrecy", "dashmap", "parking_lot", "details"] }
55+
typesize = { version = "0.1.5", optional = true, features = ["url", "time", "serde_json", "secrecy", "dashmap", "parking_lot", "nonmax", "details"] }
5556
# serde feature only allows for serialisation,
5657
# Serenity workspace crates
5758
command_attr = { version = "0.5.1", path = "./command_attr", optional = true }

examples/e05_command_framework/src/main.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -555,8 +555,11 @@ async fn slow_mode(ctx: &Context, msg: &Message, mut args: Args) -> CommandResul
555555
format!("Successfully set slow mode rate to `{slow_mode_rate_seconds}` seconds.")
556556
}
557557
} else if let Some(channel) = msg.channel_id.to_channel_cached(&ctx.cache) {
558-
let slow_mode_rate = channel.rate_limit_per_user.unwrap_or(0);
559-
format!("Current slow mode rate is `{slow_mode_rate}` seconds.")
558+
if let Some(slow_mode_rate) = channel.rate_limit_per_user {
559+
format!("Current slow mode rate is `{slow_mode_rate}` seconds.")
560+
} else {
561+
"There is no current slow mode rate for this channel.".to_string()
562+
}
560563
} else {
561564
"Failed to find channel in cache.".to_string()
562565
};

src/model/channel/attachment.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use nonmax::NonMaxU32;
12
#[cfg(feature = "model")]
23
use reqwest::Client as ReqwestClient;
34
use serde_cow::CowStr;
@@ -40,15 +41,15 @@ pub struct Attachment {
4041
/// Sescription for the file (max 1024 characters).
4142
pub description: Option<FixedString<u16>>,
4243
/// If the attachment is an image, then the height of the image is provided.
43-
pub height: Option<u32>,
44+
pub height: Option<NonMaxU32>,
45+
/// If the attachment is an image, then the width of the image is provided.
46+
pub width: Option<NonMaxU32>,
4447
/// The proxy URL.
4548
pub proxy_url: FixedString,
4649
/// The size of the file in bytes.
4750
pub size: u32,
4851
/// The URL of the uploaded attachment.
4952
pub url: FixedString,
50-
/// If the attachment is an image, then the width of the image is provided.
51-
pub width: Option<u32>,
5253
/// The attachment's [media type].
5354
///
5455
/// [media type]: https://en.wikipedia.org/wiki/Media_type
@@ -80,7 +81,7 @@ pub struct Attachment {
8081
impl Attachment {
8182
/// If this attachment is an image, then a tuple of the width and height in pixels is returned.
8283
#[must_use]
83-
pub fn dimensions(&self) -> Option<(u32, u32)> {
84+
pub fn dimensions(&self) -> Option<(NonMaxU32, NonMaxU32)> {
8485
self.width.and_then(|width| self.height.map(|height| (width, height)))
8586
}
8687

src/model/channel/embed.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use nonmax::NonMaxU32;
2+
13
use crate::internal::prelude::*;
24
use crate::model::{Colour, Timestamp};
35

@@ -171,9 +173,9 @@ pub struct EmbedImage {
171173
/// A proxied URL of the image.
172174
pub proxy_url: Option<FixedString>,
173175
/// The height of the image.
174-
pub height: Option<u32>,
176+
pub height: Option<NonMaxU32>,
175177
/// The width of the image.
176-
pub width: Option<u32>,
178+
pub width: Option<NonMaxU32>,
177179
}
178180

179181
/// The provider of an embed.
@@ -203,9 +205,9 @@ pub struct EmbedThumbnail {
203205
/// A proxied URL of the thumbnail.
204206
pub proxy_url: Option<FixedString>,
205207
/// The height of the thumbnail in pixels.
206-
pub height: Option<u32>,
208+
pub height: Option<NonMaxU32>,
207209
/// The width of the thumbnail in pixels.
208-
pub width: Option<u32>,
210+
pub width: Option<NonMaxU32>,
209211
}
210212

211213
/// Video information for an embed.
@@ -220,7 +222,7 @@ pub struct EmbedVideo {
220222
/// A proxied URL of the thumbnail.
221223
pub proxy_url: Option<FixedString>,
222224
/// The height of the video in pixels.
223-
pub height: Option<u32>,
225+
pub height: Option<NonMaxU32>,
224226
/// The width of the video in pixels.
225-
pub width: Option<u32>,
227+
pub width: Option<NonMaxU32>,
226228
}

src/model/channel/guild_channel.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ use std::fmt;
22
#[cfg(feature = "model")]
33
use std::sync::Arc;
44

5+
use nonmax::{NonMaxU16, NonMaxU32, NonMaxU8};
6+
57
#[cfg(feature = "model")]
68
use crate::builder::{
79
Builder,
@@ -44,7 +46,7 @@ pub struct GuildChannel {
4446
/// The bitrate of the channel.
4547
///
4648
/// **Note**: This is only available for voice and stage channels.
47-
pub bitrate: Option<u32>,
49+
pub bitrate: Option<NonMaxU32>,
4850
/// The Id of the parent category for a channel, or of the parent text channel for a thread.
4951
///
5052
/// **Note**: This is only available for channels in a category and thread channels.
@@ -88,7 +90,7 @@ pub struct GuildChannel {
8890
/// The maximum number of members allowed in the channel.
8991
///
9092
/// **Note**: This is only available for voice channels.
91-
pub user_limit: Option<u32>,
93+
pub user_limit: Option<NonMaxU8>,
9294
/// Used to tell if the channel is not safe for work. Note however, it's recommended to use
9395
/// [`Self::is_nsfw`] as it's gonna be more accurate.
9496
// This field can or can not be present sometimes, but if it isn't default to `false`.
@@ -99,7 +101,7 @@ pub struct GuildChannel {
99101
/// **Note**: This is only available for text channels excluding news channels.
100102
#[doc(alias = "slowmode")]
101103
#[serde(default)]
102-
pub rate_limit_per_user: Option<u16>,
104+
pub rate_limit_per_user: Option<NonMaxU16>,
103105
/// The region override.
104106
///
105107
/// **Note**: This is only available for voice and stage channels. [`None`] for voice and stage
@@ -109,14 +111,12 @@ pub struct GuildChannel {
109111
pub video_quality_mode: Option<VideoQualityMode>,
110112
/// An approximate count of messages in the thread.
111113
///
112-
/// This is currently saturated at 255 to prevent breaking.
113-
///
114114
/// **Note**: This is only available on thread channels.
115-
pub message_count: Option<u32>,
115+
pub message_count: Option<NonMaxU32>,
116116
/// An approximate count of users in a thread, stops counting at 50.
117117
///
118118
/// **Note**: This is only available on thread channels.
119-
pub member_count: Option<u8>,
119+
pub member_count: Option<NonMaxU8>,
120120
/// The thread metadata.
121121
///
122122
/// **Note**: This is only available on thread channels.
@@ -138,7 +138,7 @@ pub struct GuildChannel {
138138
pub flags: ChannelFlags,
139139
/// The number of messages ever sent in a thread, it's similar to `message_count` on message
140140
/// creation, but will not decrement the number when a message is deleted.
141-
pub total_message_sent: Option<u64>,
141+
pub total_message_sent: Option<NonMaxU32>,
142142
/// The set of available tags.
143143
///
144144
/// **Note**: This is only available in forum channels.
@@ -157,7 +157,7 @@ pub struct GuildChannel {
157157
/// is copied to the thread at creation time and does not live update.
158158
///
159159
/// **Note**: This is only available in a forum or text channel.
160-
pub default_thread_rate_limit_per_user: Option<u16>,
160+
pub default_thread_rate_limit_per_user: Option<NonMaxU16>,
161161
/// The status of a voice channel.
162162
///
163163
/// **Note**: This is only available in voice channels.

src/model/channel/message.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use std::fmt::Display;
55
#[cfg(all(feature = "cache", feature = "model"))]
66
use std::fmt::Write;
77

8+
use nonmax::NonMaxU64;
9+
810
#[cfg(all(feature = "model", feature = "utils"))]
911
use crate::builder::{Builder, CreateAllowedMentions, CreateMessage, EditMessage};
1012
#[cfg(all(feature = "cache", feature = "model"))]
@@ -123,7 +125,7 @@ pub struct Message {
123125
/// A generally increasing integer (there may be gaps or duplicates) that represents the
124126
/// approximate position of the message in a thread, it can be used to estimate the relative
125127
/// position of the message in a thread in company with total_message_sent on parent thread.
126-
pub position: Option<u64>,
128+
pub position: Option<NonMaxU64>,
127129
/// Data of the role subscription purchase or renewal that prompted this
128130
/// [`MessageType::RoleSubscriptionPurchase`] message.
129131
pub role_subscription_data: Option<RoleSubscriptionData>,

src/model/event.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// Just for MessageUpdateEvent (for some reason the #[allow] doesn't work when placed directly)
77
#![allow(clippy::option_option)]
88

9+
use nonmax::NonMaxU64;
910
use serde::de::Error as DeError;
1011
use serde::Serialize;
1112

@@ -533,7 +534,7 @@ pub struct MessageUpdateEvent {
533534
pub thread: Option<Option<Box<GuildChannel>>>,
534535
pub components: Option<FixedArray<ActionRow>>,
535536
pub sticker_items: Option<FixedArray<StickerItem>>,
536-
pub position: Option<Option<u64>>,
537+
pub position: Option<Option<NonMaxU64>>,
537538
pub role_subscription_data: Option<Option<RoleSubscriptionData>>,
538539
pub guild_id: Option<GuildId>,
539540
pub member: Option<Option<Box<PartialMember>>>,

src/model/gateway.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Models pertaining to the gateway.
22
3-
use std::num::NonZeroU16;
3+
use std::num::{NonZeroU16, NonZeroU64};
44

55
use serde::ser::SerializeSeq;
66
use url::Url;
@@ -402,8 +402,8 @@ impl serde::Serialize for ShardInfo {
402402
#[derive(Clone, Debug, Deserialize, Serialize)]
403403
#[non_exhaustive]
404404
pub struct ActivityTimestamps {
405-
pub end: Option<u64>,
406-
pub start: Option<u64>,
405+
pub end: Option<NonZeroU64>,
406+
pub start: Option<NonZeroU64>,
407407
}
408408

409409
bitflags! {

src/model/guild/audit_log/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use std::mem::transmute;
44

5+
use nonmax::{NonMaxU32, NonMaxU64};
56
use serde::ser::{Serialize, Serializer};
67

78
mod change;
@@ -368,16 +369,16 @@ pub struct Options {
368369
pub application_id: Option<ApplicationId>,
369370
/// Number of days after which inactive members were kicked.
370371
#[serde(default, with = "optional_string")]
371-
pub delete_member_days: Option<u64>,
372+
pub delete_member_days: Option<NonMaxU32>,
372373
/// Number of members removed by the prune
373374
#[serde(default, with = "optional_string")]
374-
pub members_removed: Option<u64>,
375+
pub members_removed: Option<NonMaxU64>,
375376
/// Channel in which the messages were deleted
376377
#[serde(default)]
377378
pub channel_id: Option<ChannelId>,
378379
/// Number of deleted messages.
379380
#[serde(default, with = "optional_string")]
380-
pub count: Option<u64>,
381+
pub count: Option<NonMaxU64>,
381382
/// Id of the overwritten entity
382383
#[serde(default)]
383384
pub id: Option<GenericId>,

src/model/guild/audit_log/utils.rs

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,27 +41,69 @@ pub mod webhooks {
4141
/// Used with `#[serde(with = "optional_string")]`.
4242
pub mod optional_string {
4343
use std::fmt;
44+
use std::marker::PhantomData;
45+
use std::str::FromStr;
4446

4547
use serde::de::{Deserializer, Error, Visitor};
4648
use serde::ser::Serializer;
4749

48-
pub fn deserialize<'de, D: Deserializer<'de>>(
49-
deserializer: D,
50-
) -> Result<Option<u64>, D::Error> {
51-
deserializer.deserialize_option(OptionalStringVisitor)
50+
// Workaround for https://github.com/LPGhatguy/nonmax/issues/17
51+
pub(crate) trait TryFromU64
52+
where
53+
Self: Sized,
54+
{
55+
type Err: fmt::Display;
56+
fn try_from_u64(value: u64) -> Result<Self, Self::Err>;
57+
}
58+
59+
impl TryFromU64 for u64 {
60+
type Err = std::convert::Infallible;
61+
fn try_from_u64(value: u64) -> Result<Self, Self::Err> {
62+
Ok(value)
63+
}
64+
}
65+
66+
impl TryFromU64 for nonmax::NonMaxU64 {
67+
type Err = nonmax::TryFromIntError;
68+
fn try_from_u64(value: u64) -> Result<Self, Self::Err> {
69+
Self::try_from(value)
70+
}
71+
}
72+
73+
impl TryFromU64 for nonmax::NonMaxU32 {
74+
type Err = nonmax::TryFromIntError;
75+
fn try_from_u64(value: u64) -> Result<Self, Self::Err> {
76+
Self::try_from(u32::try_from(value)?)
77+
}
78+
}
79+
80+
pub fn deserialize<'de, D, T>(deserializer: D) -> Result<Option<T>, D::Error>
81+
where
82+
D: Deserializer<'de>,
83+
T: FromStr + TryFromU64,
84+
<T as FromStr>::Err: fmt::Display,
85+
{
86+
deserializer.deserialize_option(OptionalStringVisitor::<T>(PhantomData))
5287
}
5388

54-
pub fn serialize<S: Serializer>(value: &Option<u64>, serializer: S) -> Result<S::Ok, S::Error> {
89+
pub fn serialize<S: Serializer>(
90+
value: &Option<impl ToString>,
91+
serializer: S,
92+
) -> Result<S::Ok, S::Error> {
5593
match value {
5694
Some(value) => serializer.serialize_some(&value.to_string()),
5795
None => serializer.serialize_none(),
5896
}
5997
}
6098

61-
struct OptionalStringVisitor;
99+
struct OptionalStringVisitor<T>(PhantomData<T>);
62100

63-
impl<'de> Visitor<'de> for OptionalStringVisitor {
64-
type Value = Option<u64>;
101+
impl<'de, T> Visitor<'de> for OptionalStringVisitor<T>
102+
where
103+
T: FromStr + TryFromU64,
104+
<T as FromStr>::Err: fmt::Display,
105+
{
106+
type Value = Option<T>;
65107

66108
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
67109
formatter.write_str("an optional integer or a string with a valid number inside")
@@ -71,7 +113,7 @@ pub mod optional_string {
71113
self,
72114
deserializer: D,
73115
) -> Result<Self::Value, D::Error> {
74-
deserializer.deserialize_any(OptionalStringVisitor)
116+
deserializer.deserialize_any(OptionalStringVisitor(PhantomData))
75117
}
76118

77119
fn visit_none<E: Error>(self) -> Result<Self::Value, E> {
@@ -83,11 +125,11 @@ pub mod optional_string {
83125
Ok(None)
84126
}
85127

86-
fn visit_u64<E: Error>(self, val: u64) -> Result<Option<u64>, E> {
87-
Ok(Some(val))
128+
fn visit_u64<E: Error>(self, val: u64) -> Result<Self::Value, E> {
129+
T::try_from_u64(val).map(Some).map_err(Error::custom)
88130
}
89131

90-
fn visit_str<E: Error>(self, string: &str) -> Result<Option<u64>, E> {
132+
fn visit_str<E: Error>(self, string: &str) -> Result<Self::Value, E> {
91133
string.parse().map(Some).map_err(Error::custom)
92134
}
93135
}

0 commit comments

Comments
 (0)