Big diff, but it’s mostly just whitespace changes; ignore whitespace and
it’s much smaller, though still not as tiny as it could potentially be.
Essentially, this just duplicates everything for the hashbrown variant.
It’d be possible to use generic associated types to achieve this without
the duplication, but that depends on currently-unstable features, and is
probably slightly more painful to use anyway. I’ll keep the approach in
mind for a possible version 2, but for now this is the pragmatic route.
The *name* UncheckedAnyExt was ending up visible in docs, but it had
become an increasingly unpleasant name. “Downcast” is suitable, though,
being private, it’s not still not perfect. But there’s no point in
making it public, as people generally can’t implement it because of
coherence rules (I tried).
Plus documentation and style changes.
As for Extend, eh, that should ideally be in a different commit, but
it’s here now, and I’m the only one working on this code base in
general, so I permit myself to be slightly lazy from time to time.
Trouble was Downcast should never have had an Any supertrait, as it was
grossly misleading, leading to type_id giving `dyn Any`’s TypeId rather
than the underlying type’s.
This was necessary in the days of the raw module, when inserting to the
raw map was the unsafe operation rather than getting mutable access to
the raw map, to which I have changed it now.
When I implemented Extend<Box<A>> for Map<A>, I thought:
can I implement Extend<(TypeId, Box<A>)> for RawMap<A>,
as HashMap<K, V> implements Extend<(K, V)>?
No, I responded, for insert is unsafe,
and a trait implementation cannot be marked unsafe.
Then said I, hang on, why is insert unsafe?
For by analogy with pointers, creating a dangling pointer is safe,
it’s only dereferencing it that’s unsafe;
so too here could insertion be safe and retrieval unsafe.
Then I realised: RawMap is actually completely safe, of itself;
the reason the unsafety is needed is AsMut<RawMap<A>> for Map<A>:
that retrieval is defined as safe, so insertion need be done delicately.
And so I consulted with myself and wondered:
Would it not be better to drop AsMut,
exposing rather an `unsafe fn as_raw_mut`?
For `AsRef<RawMap<A>>` and `Into<RawMap<A>>` may yet be safe,
yet this would take RawMap towards parity with HashMap.
And yet further went I,
descending into depths unplumbed these five years,
saying unto myself:
Wherefore RawMap<A> at all?
Why not rather HashMap<TypeId, Box<A>, BuildHasherDefault<TypeIdHasher>>,
accessed freely and safely by reference or Into,
and unsafely as discussed in the previous stanza
(if I may call it such)?
Striving to understand the matter,
it was a wearisome effort,
and I could not.
I consulted with 143ee06268,
yet with the passage of nigh seven years it was not able to tell me
just why I had thought this might be a good idea.
For lo, those were the benighted ages before Rust 1.0.0,
though the glimmer of that bright dawn danced on the horizon
like the latter part of an arctic winter.
And so, casting away the trammels of history
I forged a new path.
For lo! had I not even then declared it
“not necessarily the final form”?
Casting RawMap away from me and clasping HashMap to my bosom,
I found the required diff in lib.rs
such a delicate thing,
so slight.
It was pleasing in my eyes, and so forthwith I decided:
hew down the unwanted abstraction,
and bind it with a band of iron and bronze,
that it may grow no more.
So need I not add more features to it,
mere shadows of the true HashMap underneath.
Oh fortunate day!
Three-hundred-odd lines removed,
(though more detailed comments offset this,
so that the end result is more like 223,)
and simplicity restored.
Well, except for this fly in the ointment:
std versus hashbrown.
Woe unto the person who calls a raw map std::collections::HashMap,
for when another comes and enables hashbrown,
the first shall crumble into nothingness and errors most distressing.
The mitigation of this is `pub type RawMap<A>`,
augmented by the very truth that,
few using this feature,
few may stumble!
Yet there are difference betwixt the twain,
seen in my cfg branching on VacantEntry and OccupiedEntry,
and this *is* a very mild violation of the principle of strictly additive features.
There ’tis: the tale of an abstraction unravelled.
Unravelled? Feels more like “not ravelled” rather than undoing ravelling.
Deravelled? Disravelled?
I shall but brand the abstraction a dead princess, as it were,
and this my pavane pour un infante défunte.
And if you, dear reader—
if reader there be of this screed that has grown rather longer than originally anticipated but also probably more entertaining if you don’t mind this sort of thing or share a similar sense of humour to me—
have aught to opine on the matter,
You know my email address.
I’m quite pleased with how this has turned out.
Given the stability-despite-instability of hashbrown (that the API
surface we’re depending on hasn’t changed since 0.1.1), and the
deliberate altered SemVer guarantees for it, it was very tempting
to leave the hashbrown range open, `version = ">=0.1.1"` or at least
`version = ">=0.1.1, <1"`, but for some reason or other I ended up
deciding not to. I’m still of two minds about it, really.
Wait a few years and nice things stabilise!
• u64::from_ne_bytes([u8; 8]) is stable in 1.32.0
• TryFrom<&[u8]> for [u8; 8] is stable in 1.34.0
(There are other things I’m touching today that also require a more mild
MSRV bump, but this is the most I *need* at this time.)
Explained in the SAFETY comment. I’m not happy about *doing* this, but
it will make *using* this crate easier, since future-compatibility lints
make noise on bin crate builds, so this was polluting other people’s
code and making life harder for users.
I have traded one evil (a spurious warning) for another (unsafe code).
Turns out its commenting technique was completely broken—the attributes
have to be attached to an item *inside* the macro, not outside. And
judging by https://docs.rs/anymap/0.11.0/anymap/any/trait.CloneAny.html,
it was broken from the start, and I never noticed. Sigh. Now, you get a
warning that it’s not going to work like you want. Good stuff.
Well, that macro wasn’t a great idea anyway. Doing without it ends up a
little longer, and risks inconsistent editing, but is decidedly easier
to read.
This reverts commit 479d756c99.
There’s nothing wrong with this patch, but I had never pulled this
commit to my local repository and had completely forgotten about it, and
today removed the unsafe code in a *different* direction that I like
better (`bytes.try_into().map(|bytes| u64::from_ne_bytes(bytes))`), so
reverting it so I can cleanly rebase is just easier for me!
It was implemented on RawMap, and I’m not sure quite why it wasn’t
implemented on Map. I can’t think of any reason *not* to, though, so we
might as well.
Closes#30. Thanks to Maxwell Koo <mjkoo90@gmail.com> for the fix.
A better pattern is to put benchmarks in the `benches` directory;
that way, `cargo test` won’t pick them up by default,
and so it won’t fail on the stable and beta channels.
Somewhere along the path I didn’t mark some functions as `#[inline]`
which they should probably be.
Small but visible benchmark improvements, but within ε so low
confidence.
This *does* mean that they no longer function as tests, which was
deliberate, but rustc is just too slow with the assertions in there as
well. If I care, I can make variants of it that actually test. For now,
I’m sufficiently happy with it.
