Introduce a SearchIndex trait and SearchIndexWithLocate trait. #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are intended to be the generic public API. FMIndexBackend is now not intended to be used in the public API at all, though it's still exposed to it because the SearchIndex trait depends on it. The SearchIndex and SearchIndexWithLocate traits are object-safe, i.e. they're compatible with dyn. The idea is that we can then use a builder that produces a Box<dyn SearchIndex>, as this is required to support a dynamic builder that picks the backend based on the builder parameters. I haven't implemented this new builder yet, but this is a prerequisite for that. Unfortunately to make SearchIndex object-safe I can't use the generic K argument to `search` anymore. This is used to easily take an AsRef and is especially useful when you pass in a string. I've instead come up with a solution using dyn, which has some performance overhead but I suspect it's really tiny so we should be okay. That solution is not the exact equivalent of AsRef; you have to manually `&` the string to make a reference to make it work. I think that's acceptable enough.
|
I am going to hold off merging this one. I think in the end we need an approach quite similar to the previous work I did to avoid exposing the backend, but I think I will continue this tomorrow. |
|
I'm closing this one as the branch is a bit misnamed now (it's not necessarily dyn compatible). I'm going to put up another draft PR for my latest work. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are intended to be the generic public API. FMIndexBackend is now not intended to be used in the public API at all, though it's still exposed to it because the SearchIndex trait depends on it.
The SearchIndex and SearchIndexWithLocate traits are object-safe, i.e. they're compatible with dyn. The idea is that we can then use a builder that produces a Box, as this is required to support a dynamic builder that picks the backend based on the builder parameters. I haven't implemented this new builder yet, but this is a prerequisite for that.
Unfortunately to make SearchIndex object-safe I can't use the generic K argument to
searchanymore. This is used to easily take an AsRef and is especially useful when you pass in a string. I've instead come up with a solution using dyn, which has some performance overhead but I suspect it's really tiny so we should be okay.That solution is not the exact equivalent of AsRef; you have to manually
&the string to make a reference to make it work. I think that's acceptable enough.