Emerentius / ord_subset

Tools for working with types where a subset of values has a total order, like e.g. floats without NaN
Apache License 2.0
12 stars 1 forks source link

Fix 'where Self:' warnings by splitting 'OrdSubsetSliceExt' up into 'OrdSubsetSliceExt' and 'OrdSubsetSliceExtMut' #3

Closed expenses closed 6 years ago

expenses commented 6 years ago

This gets rid of a bunch of warnings such as:

warning: the trait `slice_ext::OrdSubsetSliceExt` cannot be made into an object ord_subset, ord_subset
  --> src/slice_ext.rs:42:5
   |
42 | /     fn ord_subset_sort(&mut self)
43 | |     where
44 | |         Self: AsMut<[T]>,
45 | |         T: OrdSubset;
   | |_____________________^
   |
   = note: #[warn(where_clauses_object_safety)] on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a      hard error in a future release!
   = note: for more information, see issue #51443 <https://github.com/rust-lang/rust/issues/51443>
   = note: method `ord_subset_sort` references the `Self` type in where clauses

The linked issue is https://github.com/rust-lang/rust/issues/51443. Unfortunately, the best solution I've been able to come up with so far is to split up OrdSubsetSliceExt into two traits, OrdSubsetSliceExt, and OrdSubsetSliceExtMut which requires AsMut<[T]>.

Emerentius commented 6 years ago

Thanks for the PR and the notice.

The trait was never intented to be used as an object, it's just there to extend [T]. The regular sort methods are all just methods on [T] without a trait, too after all. But, as it's going to break in the future anyway, no point in just allowing the warning.

I think it's best to reduce the implementors to just [T]. Any type that coerces via Deref or UnsizedCoercions like the usual suspects, arrays and vecs, will continue to work. Types that are only AsRef<T> or AsMut<T> will have to convert explicitly.

The (temporary) branch object_safety has the trait changes. I just need to adapt the tests so that they call the sort methods directly on the source types and not the slices that as_ref() or as_mut() gives.

expenses commented 6 years ago

Oh ok, cool thanks.

Emerentius commented 6 years ago

The situation here is kinda shitty. See also issue #4. The lint is a false positive, that I can't deactivate and that I can't work around without a breaking change. For now, I'm going to keep the status quo.