Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of some traits for their Deref counterparts. #139

Merged
merged 6 commits into from
Sep 29, 2020

Conversation

hdlj
Copy link
Collaborator

@hdlj hdlj commented Sep 29, 2020

Traits concerned

  • FstOp
  • FstOp2
  • TrMapper
  • FstCache

Remove specific implementation for Arc.

By this way, this traits are now available on Rc and Arc for instance.

@kali
Copy link
Collaborator

kali commented Sep 29, 2020

while on the topic, do we have a compelling reason why determinize needs an Arc and not just an &Fst?

@Garvys
Copy link
Collaborator

Garvys commented Sep 29, 2020

yes I need to find a way to avoid that. The issue is that determinize (static version) shares most of the code of the lazy version. And in the lazy implementation, the Fst to be determinized is stored as an arc for different reasons:

  • Avoid having to handle lifetime everywhere
  • If we have an &Fst, then the input fst need to stay on scope as long as the DeterminizedFst (lazy version) is on scope. By using Arc, it is done automatically.
    This is the case for other algorithms that share a big part of their code between the static version and the lazy version like the composition.

@hdlj hdlj merged commit 9bd5bef into main Sep 29, 2020
@hdlj hdlj deleted the task/fst-op-rc-arc branch September 29, 2020 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants