lots of polishing, regular maintenance, preparing for 0.4 (or 1.0?) release

[BurntSushi]

High level plans:

• Deprecate fst-regex in favor of using regex-automata. I already have a PR up on regex-automata that adds impls for fst::Automaton when the fst1 feature is enabled: cleanup + switch to GitHub Actions + add fst automata support + massive compile time improvements regex-automata#6
• Deprecate fst-levenshtein and move its code back into a fst::levenshtein sub-module.
• Move benchmarks to criterion.
• Add a type parameter to Fst, Set and Map that requires AsRef<[u8]>. Subsequently remove the dependency on memmap since callers can then just create their own memory map and provide it.
• Refactor fst-bin to use clap and bstr.
• Add API for looking up a map's key given its value. (See provide reverse mapping from value to key in some cases #16.)
• Experiment with adding a new type of stream that includes the state. i.e., Add a StreamWithState streamer #61.
• Experiment with adding an optional CRC check to FST files.
• Experiment with something like streaming-iterator instead of the existing trait definition. See: lots of polishing, regular maintenance, preparing for 0.4 (or 1.0?) release #96 (comment)

[Kerollmops]

Continuing the discussion I miss-started on a commit instead of the PR itself.

---

If a user wants to write a structure that stores a fst::Map<D> he is forced to specify that the D type is AsRef<[u8]> even if he will not be read or used but only stored.

struct CurrentStruct<D: AsRef<[u8]>> {
    fst: fst::Map<D>,
}

struct ExpectedStruct<D> {
    fst: fst::Map<D>,
}

[BurntSushi]

Sure, I understand that as a technical restriction. But why would someone want to do that? Is there a specific use case you have in mind?

[Kerollmops]

But why would someone want to do that?

If you mean: Why would someone want to wrap/store an FST inside of a struct that do not need to read the FST itself?

I think about one little thing that would not be possible if the constraints are on the declaration, for example a generic struct that just displays the size_of a wrapped type, or displays the type name of a struct.

Don't know if it make any sense but common std collections do not trait constrain at the struct level, that makes users lives easier, even if the generic types must impl a bunch of traits to make the collections useful (K: Hash, or K: Ord).

[BurntSushi]

I'll consider it, thanks. And yes, I know that the std collections don't do it, but it's not clear to me whether that was a mistake or not.

[Kerollmops]

There is many arguments defending my point in this recent issue in the Rust repo. A PR has been merged to relax those bounds. I think that it is important to be the less restrictive possible when unneeded, but that's only my point of view.

[BurntSushi]

Right, and in that PR, my cohorts on the library team brought up concerns with doing it. That's basically my stance as well. I just really haven't seen a compelling use case. Like I said, I'll consider it.

[Kerollmops]

I just wanted to see if I was irrelevant so I checked the API Guideline, the Exception point is on my side. I don't want to fight, I am just trying to make the library more flexible :)

EDIT: I wrote this before I saw your message above.

[BurntSushi]

Not really. That guideline is appropriately circumspect:

There is a grey area around other non-derivable trait bounds that are not strictly required by the structure definition, like Read or Write. They may communicate the intended behavior of the type better in its definition but also limits future extensibility. Including semantically useful trait bounds on data structures is still less problematic than including derivable traits as bounds.

We are spending too much time discussing a very minor point. Like I said, I'll consider it.

[vorner]

I was asked to have a look here, as a production user. So far:

• The planned changes don't seem to break my use case, which is nice :-)
• The streaming-iterator might actually make my code a bit nicer as it has some combinators.

It would also be really cool if fst could store other things than just u64 as the values, but that might be asking too much.

[BurntSushi]

@vorner Thanks! And yeah, I don't have any plans to extent the output value to anything beyond a u64. That's a fairly major change internally and would likely also require major API design work.

[Kerollmops]

Don't know if it is useful to talk about that in here, but the Streaming Iterator the FST rely on could be expressed using GATs when those becomes available (on stable), avoiding much of the traits tricks that are currently used.

Bumping the FST library to 1.0 is probably not a good idea now, I think it would have been better to bump it to this particular version when GATs are available (it seems there is progress) and we rely on them. If it is not something you think that will happen soon enough, so, let's bump to the stable version anyway :)

[BurntSushi]

Yes, I'm intensely aware that GATs would likely fix the problem. I've been waiting for them (or an equivalent) for five years. GATs don't appear to be anywhere close to landing. A generous estimate would be that they land on stable a year from now. Doing an fst 2.0 release at that point (or even another year after it) would be fine.

Normally I like to make major versions last for a long time, but fst is not a core ecosystem crate, so I'm generally happy to bump its major version at a bit of a higher frequency then, say, regex.

[BurntSushi]

Experiment with something like streaming-iterator instead of the existing trait definition.

As soon as I re-contextualized myself with this problem, it became clear to me that streaming-iterator (or an equivalent) is insufficient for fst. In particular, its main problem is that get() (or next()) return a &Self::Item, which is a particularly restrictive requirement. In some cases this would be okay. For example, Self::Item == [u8] in the case of a fst::set::Stream. But in many cases, we want the thing we return to be (&[u8], u64) or (&[u8], Output) or u64 or (&[u8], u64, A::State) or whatever. That's incompatible with requiring, at the trait level, that the item itself be a borrow.

This is precisely what GATs ("generic associated types") are supposed to help solve. If we had those, then we could in theory write a trait whose Item was itself generic over a (possibly 'static) lifetime.

I think I knew all of this at one point in time, which was how I arrived at the current Streamer trait definition. But it's been so long that I forgot all of it.

I don't see any obviously better solution with current Rust. So wait for GATs continues here.