foldVL doesn't work with all van Laarhoven folds

[tomjaguarpaw]

The type is

foldVL
  :: (forall f. Applicative f => (a -> f u) -> s -> f v)
  -> Fold s a

but it should be

foldVL'
  :: (forall f. (Applicative f, Contravariant f) => (a -> f a) -> s -> f s)
  -> Fold s a

I guess, like #533, the reason is to support old GHC's (or rather old bases) that don't include Contravariant. Perhaps #532 is the better choice. If optics is going to stick with 8.4 support, could the documentation be updated to explain that foldVL doesn't work with all van Laarhoven folds, and why this choice was made?

By the way, this seems to be the implementation:

foldVL' f =
  foldVL @_ @() @_ @() (\k s -> getAp (getConst (f (Const . Ap . k) s)))

[phadej]

forall f. Applicative f                                => (a -> f u) -> s -> f v
forall f. (Applicative f, Contravariant f) => (a -> f a) -> s -> f s

Are different in indices, not only constraints.

The former is (closer to the) type of traverse_

traverse_ :: (Foldable t, Applicative f) => (a -> f b) -> t a -> f ()

Arguably, to be lens compatible, there should be helpers for latter; but former types exist too in a libraries (especially not ones talking "lens" language at all).

[tomjaguarpaw]

Are different in indices, not only constraints.

They are, but those type parameters are phantom. foldVL' can also be written as

foldVL'
  :: (forall f. (Applicative f, Contravariant f) => (a -> f u) -> s -> f v)
  -> Fold s a

[phadej]

All I'm saying is that current foldVL is kind of inverse of traverseOf_, and that function is good to exist, maybe not named foldVL.

[phadej]

I'll leave it to @adamgundry and others to decide what to do, if anything.

[tomjaguarpaw]

current foldVL is kind of inverse of traverseOf_, and that function is good to exist

Yes, I agree.

[arybczak]

There's #430 that does what this PR does and also fixes #535. It wasn't merged due to a naming problem though.