Rewrite builders to take Cows

[GnomedDev]

String -> Cow<'a, str>
Vec<T> -> Cow<'a, [T]>
impl IntoIterator<...> -> Cow<'a, [T]>

[mkrasnitski]

Sorry for the late review on this. I feel like this is an ergonomics hit, since the Into<UserId> is gone.

[GnomedDev]

I dislike the Into Id signatures personally, as it ruins the strict typing of the ID types. Since this is a performance hit for a signature style I don't like, I removed it.

[mkrasnitski]

There's only a From<u64> impl on IDs. Since there's no way to implicitly cast an Id of one kind to a different one, I don't see how the strict typing is hurt. Being able to just pass in literals is a boon to ergonomics, and still guarantees that a list of multiple will all be converted into the same kind of Id.

[GnomedDev]

I'm not saying it allows for converting one type of ID to another, I believe that Id::new should be a boundary that needs explicitly typing to allow for the second of "wait, is this actually a user id".

[mkrasnitski]

As opposed to what? Not many constants will look like IDs, unless maybe we're talking about UNIX timestamps. Plus, the builder methods are named well enough that they clue you in already.

What looks better to you?

m.allowed_mentions(Am::new().users([UserId::new(110372470472613888), UserId::new(1103724704727656385), UserId::new(110372470472918472)].as_slice()));

vs

m.allowed_mentions(Am::new().users(vec![110372470472613888, 1103724704727656385, 110372470472918472]));

Another thing: calling .into on each ID is equivalent to the first example but is no longer explicit about the Id kind:

m.allowed_mentions(Am::new().users([110372470472613888.into(), 1103724704727656385.into(), 110372470472918472.into()].as_slice()));

Mapping over the entire array is technically possible, but is still longer than passing in just the array:

m.allowed_mentions(Am::new().users([110372470472613888, 1103724704727656385 110372470472918472].map(UserId::from).as_slice()));

So, both sides of the Into<Cow> are resultantly longer/noisier. Besides, aren't builders relatively short-lived objects? They aren't cached in any way, so the only real performance hit is the cost of creating a few allocations. These aren't large objects either, so those allocator calls are not going to be expensive.

[GnomedDev]

Small allocations still cause fragmentation and are just absolutely not needed. I agree this is a hit for passing u64s in, but in real bots these Ids should be in consts or config files anyway, I just believe this example shows the worst case scenario, which isn't even too bad.