What's the issue with FluentValue::Custom?
Currently FluentValue::Custom, introduced by #153, takes Box<dyn FluentType + Send>, this makes the variant !Sync, which propagates to the FluentValue enum, and up to FluentArgs, Scope and many more types.
This is an issue as it makes most translation operations (like Loader::lookup_with_args() from fluent-templates) bound to a single thread, when there's no actual restriction justifying that.
Approaches to the custom-type problem
From what I could see from the source code, there's currently no "real" support for custom values. In the case of an argument, the value is just turned into a string. If matched with a selector, it will always return false.
In this case, since we know that operations deal with comparing strings when using custom values, there's multiple approaches we can take.
- Generics (static dispatch) — Instead of using trait objects (dynamic dispatch), we can change the definition of the
Custom variant to take a new generic C that implements FluentType, that way only specific operations that strictly need Send/Sync will limit the use of FluentValue containing a non-Send/Sync custom type, compared to the current state where any Send/Sync operations are restricted.
- Add
+ Sync — Most types implement both Send and Sync and adding this additional trait bound would (re-)allow multithreaded translation operations. The downside to this approach is that non-Sync types would no longer be supported. But since non-Sync types are quite rare, it could be possible to implement this approach without much risk.
- Wrap the contents of the
Custom variant with SyncWrapper — While I'm not super familiar with how SyncWrapper works, this is an approach to consider.
I would be willing to do a PR but want to ensure what kind of approach is more fitting for that issue.
What's the issue with FluentType?
First, it describes duplicate(), and from how it's implemented and described, it feels like it's just a re-implementation of the standard Clone. For reference, there was this small discussion on the subject in the original PR, but I'm not familiar with the origins of the use for Any (and derived traits like AnyEq and the discussed AnyClone) in this crate, so there may be a justification for that. We should check if this is actually the case.
Second, it describes as_string() and as_string_threadsafe(). While I'm not certain of the origin of the need for such methods, I believe it should be separated and put in a more independent trait, e.g. IntlToString, FluentFormattable?
What's the issue with
FluentValue::Custom?Currently
FluentValue::Custom, introduced by #153, takesBox<dyn FluentType + Send>, this makes the variant!Sync, which propagates to theFluentValueenum, and up toFluentArgs,Scopeand many more types.This is an issue as it makes most translation operations (like
Loader::lookup_with_args()fromfluent-templates) bound to a single thread, when there's no actual restriction justifying that.Approaches to the custom-type problem
From what I could see from the source code, there's currently no "real" support for custom values. In the case of an argument, the value is just turned into a string. If matched with a selector, it will always return
false.In this case, since we know that operations deal with comparing strings when using custom values, there's multiple approaches we can take.
Customvariant to take a new genericCthat implementsFluentType, that way only specific operations that strictly needSend/Syncwill limit the use ofFluentValuecontaining a non-Send/Synccustom type, compared to the current state where anySend/Syncoperations are restricted.+ Sync— Most types implement bothSendandSyncand adding this additional trait bound would (re-)allow multithreaded translation operations. The downside to this approach is that non-Synctypes would no longer be supported. But since non-Synctypes are quite rare, it could be possible to implement this approach without much risk.Customvariant withSyncWrapper— While I'm not super familiar with howSyncWrapperworks, this is an approach to consider.I would be willing to do a PR but want to ensure what kind of approach is more fitting for that issue.
What's the issue with
FluentType?First, it describes
duplicate(), and from how it's implemented and described, it feels like it's just a re-implementation of the standardClone. For reference, there was this small discussion on the subject in the original PR, but I'm not familiar with the origins of the use forAny(and derived traits likeAnyEqand the discussedAnyClone) in this crate, so there may be a justification for that. We should check if this is actually the case.Second, it describes
as_string()andas_string_threadsafe(). While I'm not certain of the origin of the need for such methods, I believe it should be separated and put in a more independent trait, e.g.IntlToString,FluentFormattable?