Skip to content

Remove all orphan instances#53

Merged
treeowl merged 1 commit intolspitzner:masterfrom
treeowl:kill-orphans
Dec 8, 2021
Merged

Remove all orphan instances#53
treeowl merged 1 commit intolspitzner:masterfrom
treeowl:kill-orphans

Conversation

@treeowl
Copy link
Copy Markdown
Collaborator

@treeowl treeowl commented Dec 7, 2021

  • Remove all orphan instances.

  • One of the unions definitions used foldl; make it use foldl'
    instead.

Closes #52.

@treeowl treeowl force-pushed the kill-orphans branch 2 times, most recently from df564e6 to d4b9aeb Compare December 7, 2021 23:46
@treeowl treeowl requested a review from konsumlamm December 7, 2021 23:49
@treeowl treeowl force-pushed the kill-orphans branch 2 times, most recently from b4ed3d1 to f91f1a0 Compare December 8, 2021 00:00
@treeowl
Copy link
Copy Markdown
Collaborator Author

treeowl commented Dec 8, 2021

@konsumlamm This PR moves a lot of things around, so I'm pretty much blocked on everything else till it goes in.

Copy link
Copy Markdown
Collaborator

@konsumlamm konsumlamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the Functor, Fodlable and Traversable instances of Down used anywhere? Also, I think a better module name instead of Data.PQueue.Prio.Max.Down would be Data.PQueue.Down, since it's not specific to MaxQueue or MaxPQueue.

Comment thread src/Data/PQueue/Prio/Max/Internals.hs
Comment thread src/Data/PQueue/Prio/Max.hs
Comment thread src/Data/PQueue/Prio/Max.hs
@treeowl
Copy link
Copy Markdown
Collaborator Author

treeowl commented Dec 8, 2021

Are the Functor, Fodlable and Traversable instances of Down used anywhere?

I don't know.

Also, I think a better module name instead of Data.PQueue.Prio.Max.Down would be Data.PQueue.Down, since it's not specific to MaxQueue or MaxPQueue.

Good point.

* Remove all orphan instances.

* One of the `unions` definitions used `foldl`; make it use `foldl'`
  instead.

* Remove false caveat about amortized bounds. They definitely *do*
  hold in a persistent setting, and we're very careful to maintain
  that! Someone must have confused lazy-spined binomial queues with
  strict-spined ones.
@treeowl
Copy link
Copy Markdown
Collaborator Author

treeowl commented Dec 8, 2021

Traversable Down wasn't used, so I removed it. Functor Down and Foldable Down are currently used. We probably don't actually want to use Foldable Down (see #55), but we'll deal with that in a different PR. The Functor instance is used by things like mapKeys and mapKeysMonotonic. These are pretty inefficient no matter what we do (short of making all the trees lazy, which I'd very much rather not do), and I'm not optimistic that we can improve them much by reducing dictionary costs, but I guess we could try. I renamed the Down module Data.PQueue.Internal.Down, since it really is an internal thing.

@treeowl treeowl merged commit 33f2a68 into lspitzner:master Dec 8, 2021
@treeowl treeowl deleted the kill-orphans branch December 8, 2021 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove orphan instances

2 participants