search

haskell / core-libraries-committee

95 stars 16 forks source link

base API changes introduced by GHC proposal 475 #162

Closed tek closed 1 year ago

tek commented 1 year ago

GHC proposal 475 adds the extension ListTuplePuns that changes the source types for tuples to be data Tuple<n> a b c ... and disallows tuple syntax (a, b, c) in types when disabled (it is enabled by default).

The implementation can be examined here: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/8820

This includes a few changes to base, for which we are asking the committee for approval. The changes consist of:

Impact assessment

Hackage contains open imports of GHC.Tuple:

No mentions of GHC.Prelude anywhere.

constraint-tuples defines the same CTuple<n> classes, but doesn't import `GHC.Exts.

Declarations of CUnit in:

FerryCore cspretty disco

None of these import GHC.Exts or GHC.Classes.

No matches for \bSum\d*#, \bTuple\d*#, \bUnit# or \bSolo#.

Quite a few libraries using Tuple<n>, mostly custom preludes and their consumers.

Bodigrim commented 1 year ago

It seems to be the case that users, interested in non-punning syntax, are supposed to use these new GHC.Prelude and GHC.Tuple freely, and they are safe and reasonably stable. If this assessment is correct, I'd rather not put them under GHC.* subtree. I'd suggest Data.NonPunning.Prelude or similar.

Otherwise, if these entites are highly experimental and unstable, not suitable for general public, why not export them from ghc-prim instead of base?

tek commented 1 year ago

As I understand it, GHC.Prelude is supposed to be a general addition for the purpose of working with GHC internals, orthogonal to the proposal's primary content.

The only substantial reference to this in the discussion that I found is: https://github.com/ghc-proposals/ghc-proposals/pull/475#issuecomment-999619793

Maybe @monoidal can chime in since Richard references him as the inspiration for this addition.

Regarding GHC.Tuple, the reason it's not in ghc-prim is that it has dependencies on other base stuff. As for the module name, I do not know that anyone has strong opinions about whether it should be in GHC.*, though there is at least the reasoning that GHC.Tuple existed before and has exported Solo, which is also part of this proposal.

monoidal commented 1 year ago

I don't have much to add besides the linked comment.

The idea is that the types (,), [], * are available in Prelude, while their nonpunned versions are spread across GHC.Tuple, GHC.List, Data.Kind. Having to import 3 modules just to get the basic types is inconvenient.

In the long term, I'd like GHC.Prelude (perhaps under a different name) to be the approved way of using nonpunning syntax. But for the first releases, IMO it should be considered experimental. Perhaps ghc-prim would be the better package, but because the definition of tuples needs GHC.TypeNats and TypeError we can't use it.

hasufell commented 1 year ago

IMO it should be considered experimental.

What does "experimental" mean? Will not follow PVP? Will change API at will? Shall not be under CLC purview?

Bodigrim commented 1 year ago

GHC.Prelude strikes me as a wrong name for non-punning entities. I would never guess that this is a place to look for them. There is already compiler/GHC/Prelude.hs.

I'd imagine Data.NonPunning or Prelude.NonPunning would be more suitable names. It would be even better to find a positive characterisation instead of a negative one.

If this functionality is highly experimental yet, I'd say it's premature to think about convenience module with re-exports, and GHC.Tuple, GHC.List and Data.Kind are good enough for the proof of concept.

@tek could you please give us more details and motivation for the new entities? Which of them are "magical" and cannot be implemented outside of boot libraries? Which of them cannot be put into ghc-prim? What prevents us from putting them into a new ghc-junk / experimental-base / dependent-haskell boot package? In-tree boot packages are cheap.

Bodigrim commented 1 year ago

I skimmed through the MR, and it seems to me that there is nothing "magical" in GHC.Tuple: all these type families are for added convenience only, and can be implemented separately of the GHC proposal, potentially in a new package. Is my assessment wrong?

You've done an excellent job implementing the bulk of the GHC proposal; I suggest to leave out GHC.Prelude and GHC.Tuple out of scope for now. One can implement them outside of base, gather feedback and merge back when it gets less experimental.

nomeata commented 1 year ago

They might be a tiny bit magical because with punning off, GHC uses them for pretty-printing, and I assume that involves making them well-known entities to GHC.

adamgundry commented 1 year ago

I think there is something broken with our processes if the GHC Steering Committee has approved a proposal but it is then blocked on also gaining Core Libraries Committee approval for the same changes. I strongly believe we need some form of #145 so that new modules created by GHC developers (including but not limited to those approved via ghc-proposals) can be placed in an internal package (ghc-base, experimental-base or whatever you want to call it). The CLC can then decide about reexporting those modules/definitions from base independently.

hasufell commented 1 year ago

I think there is something broken with our processes if the GHC Steering Committee has approved a proposal but it is then blocked on also gaining Core Libraries Committee approval for the same changes.

Yes, the steering committee should have reached out before making a decision that affects base.

tomjaguarpaw commented 1 year ago

so that new modules created by GHC developers (including but not limited to those approved via ghc-proposals) can be placed in an internal package

Do you mean this can't be done now for some reason? Could you elaborate?

Bodigrim commented 1 year ago

I think there is something broken with our processes if the GHC Steering Committee has approved a proposal but it is then blocked on also gaining Core Libraries Committee approval for the same changes.

Indeed it would be nice to improve communications within GHC Steering Committee. Back in 2021 a chair of GHC Steering Committee asked CLC to assume responsibility for all changes to base (see detailed chronology). The GHC proposal in question has been approved several months later, on Apr 7, 2022, without any consultation with CLC. Complaining that CLC is somehow at fault here strikes me as deeply unfair: you are essentially unhappy that the rules you asked for are being applied to you.

@adamgundry if you have any further questions regarding the policy, please consult with the leadership of GHC Steering Committee and raise a separate issue to discuss.

I strongly believe we need some form of #145 so that new modules created by GHC developers (including but not limited to those approved via ghc-proposals) can be placed in an internal package (ghc-base, experimental-base or whatever you want to call it). The CLC can then decide about reexporting those modules/definitions from base independently.

The new entities proposed here can be put into a new boot package experimental-base, dependent-base or whatever today. This is orthogonal to and not blocked by #145, which proposes a major reshuffling of existing entities.

They might be a tiny bit magical because with punning off, GHC uses them for pretty-printing, and I assume that involves making them well-known entities to GHC.

@nomeata Interesting, thanks. What kind of pretty-printing are we talking about? Surely instance Show cannot depend on extensions enabled? But yet again it looks to me more like a convenience, which can potentially be refined later, when we gain more experience in this aread, than a crucial feature of the proposal.

I'd like to encourage proponents of the change to provide more details about stability of the feature. How likely is it to be redesigned soon? In which directions?

tomjaguarpaw commented 1 year ago

FWIW I didn't read @adamgundry's message as assigning fault. Instead I interpreted it as a general comment about a potential change that could help our community.

tek commented 1 year ago

@nomeata Interesting, thanks. What kind of pretty-printing are we talking about? Surely instance Show cannot depend on extensions enabled? But yet again it looks to me more like a convenience, which can potentially be refined later, when we gain more experience in this aread, than a crucial feature of the proposal.

The pretty-printing is done by Outputable, which can depend on extensions.

For example, we print:

Couldn't match expected type ‘Tuple (Int, Double)’

instead of

Couldn't match expected type ‘Tuple2 Int Double’

and with the extension enabled, it will of course remain (Int, Double).

This could indeed be reverted and refined later.

I don't know of any intention to keep the option open to change either of the printing schema or those families in the future, but I'm not an authority on that.

Bodigrim commented 1 year ago

@tek In such case I'd suggest you leave changes to base out of scope of your branch for now and merge the rest lest it accumulates merge conflicts. This would give us enough leeway to think about a proper place for non-punning entities without blocking your work and contributions indefinitely.

simonpj commented 1 year ago

There seem to be two conversations here:

  1. A "big-picture" conversation. How do we better align the GHC Steering Committee and the Core Libraries Committee. Clearly things can and should be better. I think it is not productive to assign blame. Better just to work together to make things better.

  2. A narrower conversation about this particular MR. I think we all want to get @tek unblocked. What is the quickest way to do that?

I suggest we continue (2) here, and take (1) elsewhere.

Where would be a good place to discuss (1)? On a CLC issue? Or a GHC Steering Committee issue? We should not let it drop.

simonpj commented 1 year ago

Returning to (2), let's see if I understand right.

Bodigrim commented 1 year ago
  • The MR proposes a purely additive change to base. Correct?

Essentially yes. There seems to be non-additive changes in Data.Typeable.Internal, which I don't understand in full, but I imagine it's minor.

  • The CLC is rightly cautious about even additive changes, becuase they carry a future maintenance burden; especially if they are somewhat experimental and perhaps might change in the light of experience. Correct?

If my understanding of the mood is right, it's not like CLC says hard no. We are just out of the context of Dependent Haskell proposals and cannot guess how experimental this stuff is. It seems that @tek cannot determine it either, which leaves us with a conservative estimate that it is all in flux.

It would be most helpful if GHC Steering Committee could enlighten us. E. g., if GHC SC thinks that this corner of DH design has been thoroughly explored and is deemed reasonably stable, I personally would be absolutely fine with including it into base.

See #143 for a recent example of an addition (incidentally again involving tuples), which we decided to hibernate until GHC design is settled: a GHC proposal to redesign class HasField has been raised just while we've been discussing providing some HasField-related instances from base. Same here: we'd look stupid if we approve this proposal only to learn next day that another grand design for non-punning entities is in works.

  • Therefore @Bodigrim wonders (above) if the additive bit could be expressed, for now at least, as a new package, perhaps dependent on base. Then the CLC would not need to be involved; when it became fully stable we could propose to move the functionality into base, but that's in the future. Question: is this approach techincally feasible?

Except one feature (namely, GHC typechecker will say Tuple2 Int Double instead of Tuple (Int, Double) - which, I imagine, should not be a blocker for an adept of DH), the proposed additive changes to base can be implemented and iterated as an independent package. If the mentioned feature is crucial, the proposed API can be implemented as a new boot package (dependent-base, experimental-base, etc.); in-tree boot packages are cheap.

tek commented 1 year ago

@Bodigrim the Typeable changes are conservative, I updated the description.

Moving GHC.Tuple into a new boot package seems sensible to me – would it be reasonable to make this the split ghc-base? It seems that would resonate with Richard's and Krzysztof's intention of making GHC.Prelude a module for GHC-specific entities.

Edit: This would only work if TypeNats were moved to ghc-base as well, not with an "initially empty ghc-base" approach.

simonpj commented 1 year ago

It would be most helpful if GHC Steering Committee could enlighten us. E. g., if GHC SC thinks that this corner of DH design has been thoroughly explored and is deemed reasonably stable, I personally would be absolutely fine with including it into base.

It's the result of a GHC Proposal, and it would not be a total surprise if we wanted to evolve it in the light of experience. To me it seems like a great example of something we would initially want to expose from ghc-base (if it existed) and later migrate to base. To me this is another argument for getting on with ghc-base!

Bodigrim commented 1 year ago

@simonpj it seems that everyone has different opinions about role and place of a potential ghc-base package. AFAIU https://github.com/haskellfoundation/tech-proposals/pull/47, the intention of the proposed ghc-base is to hide there "private, implementation-detail modules". But the proposed GHC.Prelude according to ghc-proposals#475 is meant to be public, "safe to use in ordinary code", a complement to Report-defined (well, not really, but) Prelude.

My understanding of you answer is that CLC should not deem this design explored and/or reasonably stable.

@tek a formal answer is that CLC is not an authoritative body to suggest new boot packages. My private opinion is that if I were you I'd proceed as suggested in https://github.com/haskell/core-libraries-committee/issues/162#issuecomment-1526486750, and subsequently create a new boot package dependent-base (atop of existing base, not below) to finish up outstanding parts. There are different expectations regarding a potential ghc-base, I would not steal the name yet.

gbaz commented 1 year ago

It seems to me the only thing in contestation in this proposal is adding GHC.Prelude, which would just be re-exports, correct?

So can the CLC consider accepting every part of this proposal except that, to move things forward, and leave the future question of GHC.Prelude vs. Data.NonPunning etc (as well as potential other packages) to a future discussion?

Obviously non-stable things may need to continue to live in base, and that's fine. They are still covered by the PVP, etc. They are just more likely to change in future releases.

Bodigrim commented 1 year ago

CLC can consider accepting any (precise and specific) proposal... with various outcomes. It's up to a proposer to ask and justify.

Bodigrim commented 1 year ago

@tek how would you like to proceed with this given recent decisions on ghc-experimental?

tek commented 1 year ago

The amendment of the GHC proposal to remove any changes to base is currently stalled pending the creation of ghc-experimental and a decision about modules names, but I think there is consensus that this is the way to go. If nobody has any objections, I think I can close this issue.

Bodigrim commented 1 year ago

Let me close it then, thanks all.