contain-rs / compare Goto Github PK

Convert {max, min, Extract::new} to default methods on Compare once where clauses permit equality constraints.

Blocked on rust-lang/rust#20041.

These can be copied from collect-rs.

Blocked on rust-lang/rust#7671.

Instead of using my (or @gankro's) personal access token as the value of the GH_TOKEN environment variable on Travis CI, we should create one from @FlashCat's GitHub account. This will limit the consequences of the token being compromised.

Whoever has FlashCat's GitHub credentials should do the following:

1. Log in to GitHub as FlashCat, and create a new personal access token with only the public_repo permission.
2. Copy the token to the clipboard.
3. Delete the current GH_TOKEN environment variable by visiting https://travis-ci.org/contain-rs/compare/settings/env_vars.
4. Re-create the GH_TOKEN environment variable with the token that was copied, ensuring that the "Display value in build logs" settings is "OFF" (which it is by default).
5. Repeat for each of the other contain-rs repos.

The only reason this type parameter is currently necessary is because of the design of the comparator adaptors. Without the parameter (and replacing the natural function with a unit struct), the extremely common

let cmp = Natural.rev();
assert_eq!(cmp.compare(&1, &2), Greater);

fails type inference:

error: type annotations required: cannot resolve `_ : core::cmp::Ord` [E0283]
     let cmp = Natural.rev();
                       ^~~~~

This is because the rev method is defined on the Compare trait itself, which has two type parameters (for the left- and right-hand sides of the comparison), and the specific impl to use here is unconstrained.

However, the behavior of the rev method is always the same, regardless of the concrete comparator type or the type parameters involved. Therefore, there should really only be one way (impl) to reverse a comparator, which leads to a few possible solutions. They cannot involve type parameters on Compare.

1. Use separate constructors

1. Freestanding function:

   pub struct Rev<C>(C);
   pub fn rev<C>(cmp: C) -> Rev<C>);

2. Constructor method:

   pub struct Rev<C>(C);
   impl<C> Rev<C> {
       pub fn new(cmp: C) -> Rev<C> { Rev(cmp) }
   }

3. Public field:

   pub struct Rev<C>(pub C);

This approach has the following downsides:

• The user must import an additional type or function for each adaptor. (This is potentially negated by the custom prelude RFC).

• Chaining is no longer possible, which can make things confusing. For example:

  let cmp = then(extract(SliceExt::len, rev(Natural))), extract(SliceExt::first, Natural));

  versus

  let cmp = Natural.rev().extract(SliceExt::len).then(Natural.extract(SliceExt::first));

2. Use a separate trait

pub trait CompareExt: Sized { // note that it does not extend `Compare`
    fn rev(self) -> Rev<C> { Rev(self) }
    ... // methods for other adaptors
}

1. With a blanket impl:

   impl<C> CompareExt for C {} // note that we cannot specify `C: Compare<L, R>`

2. With specific impls:

   impl CompareExt for Natural {}
   impl<C> CompareExt for Rev<C> {}
   ...

TODO: Other options.