Debias Data.Foldable somewhat
ClosedPublic

Authored by dfeuer on Nov 4 2014, 11:28 PM.

Details

Summary

Use fewer left/right-biased folds for defaults and
functions in Data.Foldable, to better support things
that don't look like cons lists.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dfeuer updated this revision to Diff 1255.Nov 4 2014, 11:28 PM
dfeuer retitled this revision from to Debias Data.Foldable somewhat.
dfeuer updated this object.
dfeuer edited the test plan for this revision. (Show Details)
dfeuer added reviewers: austin, hvr, ekmett.
dfeuer updated this revision to Diff 1256.Nov 4 2014, 11:42 PM
dfeuer edited edge metadata.

Actually use max and min. Use coerce where appropriate for
efficiency.

dfeuer updated this revision to Diff 1257.Nov 5 2014, 12:18 AM

Fix types.

dfeuer updated this revision to Diff 1258.Nov 5 2014, 12:47 AM

Silly test suite thing.

hvr requested changes to this revision.Nov 5 2014, 3:14 AM
hvr edited edge metadata.
hvr added inline comments.
libraries/base/Data/Foldable.hs
101–107

fyi, @ekmett needs to verify this Functor/Foldable law

174–203

you need to rebase this patch due to 0a8e8995fd31dd46fa9bdbc7f65984b51c8c13dc having landed already

247–248

I just realised: null = isLeft for Either (I'm not suggesting any code-change here though!)

260–305

I think Proxy is isomorphic to Const _ in terms of Foldable, so I guess the same treatment would apply to Foldable (Const m)?

277–278

A comment why we don't want to export Max/Min would be useful here; as I'm sure somebody stumbling over this will wonder

356–357

Maybe it should be mentioned that msum is a specialised version of asum?

Also, this assumes <|> == mplus... we should write that down as a law (if it isn't already -- I have a mental TODO note about specifying all sorts of implicit laws we have to assume since the AMP&BBP got implemented)

This revision now requires changes to proceed.Nov 5 2014, 3:14 AM
dfeuer added inline comments.Nov 5 2014, 8:42 AM
libraries/base/Data/Foldable.hs
101–107

FYI, we have discussed it (see Trac #9674).

247–248

Good point! That's clearer, I think.

277–278

I'll add one.

dfeuer updated this revision to Diff 1266.Nov 5 2014, 9:08 AM
dfeuer edited edge metadata.

Rebase; unclear what caused failure.

dfeuer added inline comments.Nov 5 2014, 10:23 AM
libraries/base/Data/Foldable.hs
260–305

That instance doesn't exist, does it?

dfeuer updated this revision to Diff 1268.Nov 5 2014, 10:31 AM
dfeuer edited edge metadata.

Fix bug in null; respond to some of hvr's concerns.

dfeuer updated this revision to Diff 1273.Nov 5 2014, 4:13 PM

Back off most of the debiasing, as it turns out not to make
sense in practice. Make concat and concatMap biased, because
they should be.

dfeuer updated this revision to Diff 1274.Nov 5 2014, 5:22 PM

Fix typo.

dfeuer updated this revision to Diff 1281.Nov 5 2014, 9:10 PM

Experimentally switch to monadic stuff to see if this validates.

dfeuer updated this revision to Diff 1283.Nov 5 2014, 9:22 PM

Fix typo

Yep, it looks like something, somewhere, has a much worse (*>) than (>>), and that breaks things for ghci004. No, I have no idea what or where.

hvr added a comment.EditedNov 7 2014, 6:58 AM

@dfeuer I wouldn't read too much into ghci004-failures right now, that test-case is currently known to be wiggly

ekmett accepted this revision.Nov 7 2014, 9:53 AM
ekmett edited edge metadata.

These changes LGTM.

libraries/base/Data/Foldable.hs
423

Isn't the`asTypeOf` (f . g)` unnecessary due to the type signature?

hvr accepted this revision.Nov 8 2014, 11:14 AM
hvr edited edge metadata.
dfeuer updated this revision to Diff 1348.Nov 8 2014, 3:25 PM
dfeuer edited edge metadata.

Use a few more coercions--probably not significant, but might
improve inlining in some cases.

dfeuer updated this revision to Diff 1366.Nov 9 2014, 4:40 PM

Remove unnecessary asTypeOf as @ekmett recommends.

dfeuer updated this revision to Diff 1367.Nov 9 2014, 4:53 PM

Get rid of warning, hopefully without producing type errors.

This revision was automatically updated to reflect the committed changes.