Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dup and arr2 #308

Open
ivanperez-keera opened this issue Nov 23, 2024 · 19 comments
Open

dup and arr2 #308

ivanperez-keera opened this issue Nov 23, 2024 · 19 comments
Labels
awaits-impact-assessment Awaits an impact assessment (e. g., using https://github.com/Bodigrim/clc-stackage)

Comments

@ivanperez-keera
Copy link

ivanperez-keera commented Nov 23, 2024

Hi,

Two functions from one of Yampa's auxiliary modules have become generally useful (they were made public and they have found use in games, other applications, and other libraries):

dup :: a -> (a, a)
dup x = (x, x)
arr2 :: Arrow a => (b -> c -> d) -> a (b, c) d
arr2 = arr . uncurry

Would you consider adding them to base?

I'd say the logical place for dup would be Data.Tuple and for arr2 would be Control.Arrow.

@phadej
Copy link

phadej commented Nov 23, 2024

Looks like you proposed adding dup already long ago https://mail.haskell.org/pipermail/libraries/2018-October/029051.html, has something changed?

EDIT: Why dup :: a -> (a, a), why not dup :: Arrow a (a,a) (you are proposing arr2 as arrow lifted uncurry after all!).
It would be nice if this proposal summarized the previous discussion (and rehashed questions & answers already asked and answered).

@phadej
Copy link

phadej commented Nov 23, 2024

https://hackage.haskell.org/package/extra-1.8/docs/Data-Tuple-Extra.html has dupe :: a -> (a, a) used by e.g. ghcide.

@thielema
Copy link

thielema commented Nov 23, 2024 via email

@hasufell
Copy link
Member

Looks like you proposed adding dup already long ago https://mail.haskell.org/pipermail/libraries/2018-October/029051.html, has something changed?

I don't see any conclusion in that thread.

@parsonsmatt
Copy link

I don't think this passes the Fairbairn threshold for base

@hasufell
Copy link
Member

I don't think this passes the Fairbairn threshold for base

Why? It appears these are duplicated across many libraries, have been used for years and are primitives.

Many functions in base are not very "exciting"

@mixphix
Copy link
Collaborator

mixphix commented Nov 27, 2024

For one thing, GHC.IO.Device.dup :: IODevice a => a -> IO a already exists in base, so we will have to bikeshed the name already.

There is nothing blocking arr2 in my opinion.

@treeowl
Copy link

treeowl commented Nov 27, 2024

I don't think that's a blocker. GHC.IO.Device is not a common import.

@ivanperez-keera
Copy link
Author

ivanperez-keera commented Nov 29, 2024

Sorry, it's been 6 years and I'd completely forgotten about that thread. Here's a summary of the discussion back then, together with my comments.

Name

  • Dan Burton indicated that "there is precedent in the arrow literature for calling this function "dup".
    For example, on page 55 of this paper: http://homepages.inf.ed.ac.uk/wadler/papers/arrows-jfp/arrows-jfp.pdf"

  • Edward Kmett indicated that "dup as the name for this operation has a ton of precedent in languages like
    forth, and its length is comparable to the already smashed fst and snd."

  • Carter Shonwald commented "dup is nice, but i dont really care what we call it as long as its not
    annoying or dumbe :)"

See also comments below regarding location.

Signature / implementation

There are several possible implementations of duplicating transformations:

dup :: Arrow a => a b (b,b)
dup = id &&& id
dup :: a -> (a, a)
dup = join (,)
dup :: a -> (a, a)
dup x = (x, x)

The nice thing about the first one is that it works for all arrows, not just functions. We can still use it with functions, but we no longer need to arr dup all over the place.

Location

Obviously this would be partly dependent on the signature. If it works on arrows, then it makes sense to add it to Control.Arrow. That also avoids the name clash with existing code.

@mixphix : For one thing, GHC.IO.Device.dup :: IODevice a => a -> IO a already exists in base, so we will have to bikeshed the name already.

@treeowl : GHC.IO.Device is not a common import.

I'm not sure how uncommon it is. Here's a search: https://github.com/search?q=ghc.io.device+dup+language%3AHaskell&type=code. I don't know how to search among all packages on hackage.

Note that GHC.IO.Device also defines a custom read function that is not compatible with the one in the Read class.

If both dup and arr2 are placed in Control.Arrow, which is not exported by the Prelude, then I'd argue that it's a safe change. Here's what github lists for Haskell code that lists both GHC.IO.Device and Control.Arrow : https://github.com/search?q=ghc.io.device+Control.Arrow+language%3AHaskell&type=code . The only one that looks like it could cause an issue is base-orphans.

EDIT (fix attributions for quotes).

@treeowl
Copy link

treeowl commented Nov 29, 2024

@treeowl : For one thing, GHC.IO.Device.dup :: IODevice a => a -> IO a already exists in base, so we will have to bikeshed the name already.
@mixphix : GHC.IO.Device is not a common import.

You've mixed up the attributions for the quotes. @mixphix is the one who said the name clash might be a problem; I'm the one who pointed out that it's not a common import.

@ivanperez-keera
Copy link
Author

@treeowl My bad. Sorry about that. Fixed in the original comment.

@ivanperez-keera
Copy link
Author

Gentle nudge.

@Bodigrim
Copy link
Collaborator

Bodigrim commented Dec 5, 2024

I don't know how to search among all packages on hackage.

https://hackage-search.serokell.io/?q=%5Cbdup%5Cb
There are quite a few mentions, although more analysis is needed whether any of them imports Data.Tuple / Control.Arrow at the same time. We use https://github.com/haskell/clc-stackage for impact analysis.


dup sounds good to me, indeed the name follows DUP in Forth and such.

arr2 seems to be a non-descriptive name to me, I would not have guessed what it does. A better name could convince me to add it, but as it stands arr . uncurry is not that long and much more readable.

@ivanperez-keera
Copy link
Author

ivanperez-keera commented Dec 5, 2024

arr2 seems to be a non-descriptive name to me

I think it's quite common for functions that operate on larger tuples to have a number indicating the arity. For example, in https://hackage.haskell.org/package/raft-0.3.7.2/docs/Data-Tuple-Util.html we have fst3, first3, fst4, first4. The package extra defines similar functions (up to arity 3). The same happens in regex-tdfa, MissingH, haskoin-core. The library tuple uses a similar naming scheme (the specific function names differ).

@ivanperez-keera
Copy link
Author

What are the next steps?

@Bodigrim
Copy link
Collaborator

Bodigrim commented Dec 8, 2024

@ivanperez-keera please refer to https://github.com/haskell/core-libraries-committee/blob/main/PROPOSALS.md#the-how

It's up to proposer to decide on exact content of the proposal and prepare impact assessment. If I may suggest, given that the proposal touches frequently imported modules, you likely need a draft implementation first, because otherwise it would be impossible to prepare impact assessment.

Dear CLC members, any non-binding opinions on dup and arr2? @tomjaguarpaw @hasufell @mixphix @velveteer @parsonsmatt @angerman

@tomjaguarpaw
Copy link
Member

They seem fine to me and I can see how they help people who use arrows. I agree that dup doesn't seem to pass the Fairbairn threshold versus \x -> (x, x), but I have no objection to it being exported in Control.Arrow specifically if arrow-using people like it. Indeed, I wouldn't be surprised if the impact assessment suggests that it will cause less breakage to export them both from Control.Arrow, and leave Data.Tuple alone.

@mixphix
Copy link
Collaborator

mixphix commented Dec 9, 2024

These seem like useful arrow-related functions, I think exporting them from Control.Arrow (with dup at its arrow type Arrow arr => arr a (a, a)) would be fine.

@Bodigrim Bodigrim added the awaits-impact-assessment Awaits an impact assessment (e. g., using https://github.com/Bodigrim/clc-stackage) label Dec 14, 2024
@angerman
Copy link

I'm for an export from Control.Arrow, as @tomjaguarpaw and @mixphix also seem to prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaits-impact-assessment Awaits an impact assessment (e. g., using https://github.com/Bodigrim/clc-stackage)
Projects
None yet
Development

No branches or pull requests

10 participants