spl / dlist

Difference lists in Haskell
https://hackage.haskell.org/package/dlist
BSD 3-Clause "New" or "Revised" License
65 stars 15 forks source link

Restore `tail :: DList a -> DList a`? #98

Closed andreasabel closed 3 years ago

andreasabel commented 3 years ago

In 1.0, the function tail changed to DList a -> [a], motivated by the removal of the recursor list.

However, what speaks against the following definition?

tail :: DList a -> DList a
tail (UnsafeDList f) = UnsafeDList (Data.List.tail . f)

This would be a proper efficient tail function for DList. The new tail (dlist-1.0) leaves the realm of DLists, so it seems it is already subsumed by Data.List.tail . toList.

Originally posted by @andreasabel in https://github.com/spl/dlist/pull/69#r699052240

spl commented 3 years ago

When Data.List.tail fails in that case, the error does not help you trace it to the source of the problem. That's why there is the explicit error below:

tail :: DList a -> [a]
tail xs = case toList xs of
  _ : ys -> ys
  [] -> error "Data.DList.tail: empty DList"

Do you have an actual need for tail :: DList a -> DList a? I'm skeptical that it's (a) useful and (b) worth introducing the indirect error.

andreasabel commented 3 years ago

Do you have an actual need for tail :: DList a -> DList a?

No, I just noted the API change and wondered why it was necessary.

Maybe tail :: DList a -> [a] should be deprecated altogether. It is (i) a partial function with (b) a definition below the Fairbairn threshold (tail . toList) that (c) can perfectly live outside of dlist as it does not access the internal definition of DList.

I found that errors like Data.DList.tail: empty DList are hard to track down in a large codebase, and partial functions like tail are a bit of a historical mistake (see e.g. https://github.com/agda/agda/pull/5387).

spl commented 3 years ago

In general, I agree with everything you said. But I think the error here is slightly better than the error you'd get from Data.List.tail, because you know the source was a DList and not a list. 😁

Thanks for the feedback. Always appreciated!

andreasabel commented 3 years ago

Do you have an actual need for tail :: DList a -> DList a?

No, I just noted the API change and wondered why it was necessary.

Actually, I was wrong. Indeed, package listlike repackages DList into a generic interface for list-like structures, and the loss of tail :: DList a -> DList a has to be compensated there: https://github.com/ddssff/listlike/blob/b43111c4764c5f0c4f5e4338f48481a9ff4d3536/src/Data/ListLike/DList.hs#L37-L41 Note that the compensation fromList . DLIst.tail is inefficient and could be helped by my above efficient variant.
Atm, users of the DList instance of ListLike get an inefficient tail when they expect an efficient one.

andreasabel commented 3 years ago

Let me allow another stab at this issue. I would also be grateful if you kept the issue open so that it is more visible to the community, and others can join the discussion.

I am deeply convicted that tail should be a (partial) endo-function on list structures t, so its type would be t a -> t a, and in the present case, DList a -> DList a. There are strong reasons:

I understand that it is inconvenient to row back on the change of tail, but I think for the future of dlist this would be a great decision.

Bodigrim commented 3 years ago

From my perspective, if tail is deemed to be an antipattern because of its inefficiency, it would be better to remove it from API altogether instead of changing return type.

andreasabel commented 3 years ago

From my perspective, if tail is deemed to be an antipattern because of its inefficiency, it would be better to remove it from API altogether instead of changing return type.

@Bodigrim: tail is efficient if implemented properly, see https://github.com/ddssff/listlike/pull/15.