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

Make the type of (&) analogous to the one of ($) #158

Closed
andreasabel opened this issue Apr 11, 2023 · 25 comments
Closed

Make the type of (&) analogous to the one of ($) #158

andreasabel opened this issue Apr 11, 2023 · 25 comments
Labels
approved Approved by CLC vote base-4.19 Implemented in base-4.19 (GHC 9.8)

Comments

@andreasabel
Copy link
Member

andreasabel commented Apr 11, 2023

Proposal: establish uniformity between ($) and (&). They are currently out of sync (base-4.18).

($) :: forall r a (b :: TYPE r). (a -> b) -> (a -> b)
(&) :: a -> (a -> b) -> b

I think when ($) was generalized to its present form, (&) was forgotten.

Screenshot 2023-04-11 at 16 28 52

Proposal: change to

(&) :: forall r a (b :: TYPE r). a -> (a -> b) -> b

Note: #132 generalizes the type of ($) even further, but @phadej thinks the same is not possible for (&):

Disclaimer: I'll probably won't make a MR myself.

@hasufell
Copy link
Member

Note: this would keep the existing implementation.

x & f = f x

@Bodigrim
Copy link
Collaborator

I do not expect much fight over this, seems a very reasonable addition, but there is no way forward unless someone creates an MR and completes an impact assessment.

@Bodigrim Bodigrim added the awaits-MR No GHC MR (https://gitlab.haskell.org/ghc/ghc/-/merge_requests) has been raised yet label Apr 19, 2023
@mixphix
Copy link
Collaborator

mixphix commented Apr 23, 2023

@andreasabel The proposal process is outlined in this very repository. Could you please describe why you chose to ignore it when making this proposal?

@andreasabel
Copy link
Member Author

@mixphix: As this very issue demonstrates, the process is somewhat broken, if it manages to pass a generalization of ($) from (a -> b) -> a -> b to the more polymorphic forall r a (b :: TYPE r). (a -> b) -> (a -> b) without doing the same for the twin (&). I think you can't replace expertise by a social process and voting. You need experts keeping an oversight over the big picture who can keep the library consistent.
Personally, atm I do not have the time to do this service, and I also do not have the time to become an expert for making and evaluating changes to base, which involves building GHC etc. I am an expert on other things and this is where I contribute to the Haskell community.
I still take the liberty to point out issues like this one (#158) to the responsible body.
Thanks to the CLC who have taken the responsibility for base!

@hasufell
Copy link
Member

@andreasabel You're making several assumptions:

  1. the types can be symmetric (they can't)
  2. CLC values symmetry above all else (likely false as well)

Insulting volunteers will not get you bonus points for a sub-par proposal.

@andreasabel
Copy link
Member Author

Insulting volunteers will not get you bonus points for a sub-par proposal.

I am not insulting volunteers, I am just not a believer that everything can be channeled through one process. I think you need a some kind of fast-track for such fixes, and some oversee that keeps the API consistent.

@hasufell
Copy link
Member

I think you need a some kind of fast-track for such fixes, and some oversee that keeps the API consistent.

Again you're making assumptions.

@andreasabel
Copy link
Member Author

Again you're making assumptions.

@hasufell : Sorry for making assumptions, that's human nature, I suppose. I am happy to read your personal CLC mission statement so I make less assumptions about your intentions.

Not sure if this discussion leads somewhere this way.

I don't really have anything to add. If you think this proposal is "right", then take it, otherwise leave it.

@hasufell
Copy link
Member

I am happy to read your personal CLC mission statement so I make less assumptions about your intentions.

https://gist.github.com/hasufell/d9d33cc44f65163f493e09da311ef6a7

Not sure if this discussion leads somewhere this way.

Agreed. I propose to close this proposal due to lack of a champion (the issue creator indicated they won't create an MR).

@treeowl
Copy link

treeowl commented Apr 24, 2023

It's premature to close.

@hasufell
Copy link
Member

It's premature to close.

Are you volunteering to champion the proposal?

@konsumlamm
Copy link
Contributor

I'd be happy to create an MR for this, if noone else does.

@mixphix
Copy link
Collaborator

mixphix commented Apr 24, 2023

Thank you @konsumlamm for your generous offer.

@Bodigrim
Copy link
Collaborator

As this very issue demonstrates, the process is somewhat broken, if it manages to pass a generalization of ($) from (a -> b) -> a -> b to the more polymorphic forall r a (b :: TYPE r). (a -> b) -> (a -> b) without doing the same for the twin (&). I think you can't replace expertise by a social process and voting. You need experts keeping an oversight over the big picture who can keep the library consistent.

@andreasabel I'm not sure what's implied here. ($) has always been representationally-polymorphic in b (see ghc#5570, ghc#8739, ghc/ghc@5dd1cbb), it's just that its surface type in Haddock / ghci was a lie until base-4.12 (see ghc/ghc@321b420).

Putting responsibility for decade-old decisions made by experts on the current CLC seems at the very best unwise.

I still take the liberty to point out issues like this one (#158) to the responsible body.

If you want just to file an issue, but have no intent to fix it in the foreseeable future, I'd recommend using https://gitlab.haskell.org/ghc/ghc/-/issues.

It's reasonable to prioritise proposals with willing contributors over non-proposals / open-ended issues. The truth, however, is that we'll never get time for the second category, so there is no point to stockpile unactionable items.

I'd be happy to create an MR for this, if noone else does.

@konsumlamm could you please confirm that you are willing to create an MR and make an impact assessment as well?

@konsumlamm
Copy link
Contributor

konsumlamm commented Apr 25, 2023

MR: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10338.

I decided to make r a specified type variable, as opposed to an inferred type variable, for symmetry with ($). This is technically a breaking change, however it's very unlikely to actually break something (it would only break uses of (&) with TypeApplications, there doesn't seem to be any such use on Hackage, according to https://hackage-search.serokell.io/?q=%5C%28%26%5C%29+%40). Making r an inferred type variable should not be a breaking change.

I can also try to make an impact assessment, when I get the time (hopefully this week).

@mixphix mixphix added awaits-impact-assessment Awaits an impact assessment (e. g., using https://github.com/Bodigrim/clc-stackage) and removed awaits-MR No GHC MR (https://gitlab.haskell.org/ghc/ghc/-/merge_requests) has been raised yet labels Apr 25, 2023
@andreasabel
Copy link
Member Author

@Bodigrim wrote:

If you want just to file an issue, but have no intent to fix it in the foreseeable future, I'd recommend using gitlab.haskell.org/ghc/ghc/-/issues.

Ok, thanks, I'll file issues there then, if this is the actual issue tracker for base.
I must say I am a bit confused about competencies here, I thought base was in the domain of CLC rather than GHC.
(And I was prompted by CLC to open this proposal here: #132 (comment).)

Anyway, thanks to @konsumlamm for volunteering!

@konsumlamm
Copy link
Contributor

I tried to build clc-stackage and after some trouble trying to get it working, the only error is this:

Configuring library for hw-json-simd-0.1.1.2..
Preprocessing library for hw-json-simd-0.1.1.2..
c2hs: C header contains errors:

/usr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/include/avx512fp16intrin.h:38: (column 18) [ERROR]  >>> Syntax error !
  The symbol `__v8hf' does not fit here.

Error: cabal: Failed to build hw-json-simd-0.1.1.2 (which is required by
clc-stackage-0.1.0.0). See the build log above for details.

This looks like a bug in c2hs to me, but I haven't found an existing bug report. Compiling hw-json-simd without the c2hs part works fine though (with my modified GHC).

So in summary: Everything in clc-stackage (that compiled before) compiles with the patch and I don't expect there to be any breakage (the only case I can think of that would break is using (&) with TypeApplications, which likely noone ever did).

@tbidne
Copy link

tbidne commented May 7, 2023

@konsumlamm Some more discussion on that problem here.

@Bodigrim
Copy link
Collaborator

Bodigrim commented May 8, 2023

Thanks @konsumlamm.

Dear CLC members, let's vote on the proposal to make (&) :: a -> (a -> b) -> b representation polymorphic in b:

-(&) :: a -> (a -> b) -> b
+(&) :: forall r a (b :: TYPE r). a -> (a -> b) -> b

This brings its type signature in line with ($) :: forall r a (b :: TYPE r). (a -> b) -> a -> b as in base-4.18.

(The further generalisation as for ($) in #132 is currently impossible for (&))

The MR is available at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10338/diffs. The impact assessment did not discover any breakage in the ecosystem.

@tomjaguarpaw @mixphix @chshersh @hasufell @angerman @parsonsmatt


+1 from me.

@parsonsmatt
Copy link

+1

@tomjaguarpaw
Copy link
Member

-1


If we could match ($) precisely I would be in favour even though I voted against changing ($). Given that there are technical reasons that we can't I prefer to leave it as it is. I still think this stuff belongs in a putative "lifted-base" until it's been borne out to be good in practice.

@chshersh
Copy link
Member

chshersh commented May 8, 2023

+1

1 similar comment
@hasufell
Copy link
Member

hasufell commented May 8, 2023

+1

@mixphix
Copy link
Collaborator

mixphix commented May 8, 2023

+1. I see no reason to stall nonbreaking generalizations.

@Bodigrim
Copy link
Collaborator

Bodigrim commented May 9, 2023

Thanks all, 5 votes in favour out of 7 possible are enough to approve.

@Bodigrim Bodigrim closed this as completed May 9, 2023
@Bodigrim Bodigrim added approved Approved by CLC vote and removed awaits-impact-assessment Awaits an impact assessment (e. g., using https://github.com/Bodigrim/clc-stackage) labels May 9, 2023
@Bodigrim Bodigrim added the base-4.19 Implemented in base-4.19 (GHC 9.8) label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by CLC vote base-4.19 Implemented in base-4.19 (GHC 9.8)
Projects
None yet
Development

No branches or pull requests

10 participants