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

-XUnstable, protecting GHC's unstable features #524

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

goldfirere
Copy link
Contributor

@simonpj and I brainstormed this idea about a way to have features in GHC that we know may change, but we don't want to get stuck supporting forever.

Rendered

@phadej
Copy link
Contributor

phadej commented Jul 24, 2022

Introducing, changing or removing unstable features should require a proposal nevertheless. Proposal process was set up to bring transparency to GHC development.

@phadej
Copy link
Contributor

phadej commented Jul 24, 2022

  • LinearTypes are probably unstable, but definitely required a proposal. There should haven't been a backdoor to introduce such feature as unstable without scrutinity of GHC proposal process.
  • Same applies to OverloadedRecordUpdate and OverloadedRecordDot, which are AFAIK still Unstable, but the changes to them definitely should be widely discussed.

@phadej
Copy link
Contributor

phadej commented Jul 24, 2022

Even if changes are going though GHC proposal process, I'm sceptical about this proposal:

  • What will prevent a feature being unstable for years? I.e. be in eternal "beta". Yet, it would be de-facto stable?
  • OTOH, AFAICT people knew that -XImpredicativeTypes are broken, and used it on their own risk.
    Similarly people use LinearTypes or OverloadedRecordUpdate on own risk, will requiring them to also specify -XUnstable add any value?

@Ericson2314
Copy link
Contributor

So at first glance this does look like a reincarnation of -fglasgow-exts. That is indeed a step backwards, and probably what @phadej is reacting too.

But the actual contents of the proposal is not a language feature, but marking library code (even if ghc-prim stretches the definition of a library) unstable.

I think it would be good to characterize this as something like Deprecation warnings / Safe Haskell --- a "taint system" on code. Indeed, I would like to be usable by regular users for Internals modules too.

Implementing that would go nicely with implementing #134

@Ericson2314
Copy link
Contributor

Also, there is a lot of prior art with Rust for this sort of thing. I would recommend becoming familiar with that. We have some sort of annotation system in GHC today that would allow marking individual definitions within modules, like Rust does, for this. But I understand it is little used because it depends on the template haskell interpreter.

@simonpj
Copy link
Contributor

simonpj commented Jul 26, 2022

, there is a lot of prior art with Rust for this sort of thing. I would recommend becoming familiar with that

Can you give specific links to relevant documentation? Thanks!

@simonpj
Copy link
Contributor

simonpj commented Jul 26, 2022

Several thoughts, all of which suggest changes to the proposal.

Motivation

There are actually two distinct motivations here, only one of which is discussed in the proposal:

  1. Experimental language features. This is discussed in 1.1 of the proposal.
  2. In libraries, distinguishing between GHC-internals and public API. This is not discussed in the proposal: see next Section.

The over-arching motivation (which again isn't terribly clearly stated) is this:
make it clear to programmers when they are consciously taking a dependency on something
that is unstable and likely to change.

Note: "unstable" is not the same as "unsafe". For example unsafeCoerce is unsafe, but it is very stable -- its API is not expected to change.

Distinguishing between "internals" and "public API"

Here's some code from GHC.Base

{-# RULES "map" [~1]     forall f xs. map f xs            = build (\c n -> foldr (mapFB c f) n xs)
          "mapList" [1]  forall f. foldr (mapFB (:) f) [] = map f  #-}

mapFB ::  (elt -> lst -> lst) -> (a -> elt) -> a -> lst -> lst
{-# INLINE [0] mapFB #-} -- See Note [Inline FB functions] in GHC.List
mapFB c f = \x ys -> c (f x) ys

All this is part of the list-fusion mechanism. mapFB is a purely local function. But in fact
it is exported by GHC.Base, because it might be used in, say GHC.List. So a user could say

import GHC.Base( mapFB )

and then be discombobulated when we change the implementation of fusion and abolish mapFB.

Motivation: we need a way to distinguish the "internals" of GHC from its externally-advertised API.
Programmers may be free to depend on internals, but cannot complain if they change. So they should
say something very explicit if they are going to take such a dependency; it should not be accidental.

Note:

  • Something may start as an "internal" but later migrate to be stable. For example, the SORT type constructor of Type vs Constraint proposal #518 is (in that proposal) definitely internal. But it might later become part of the advertised API of Haskell.

  • The "public API" part of GHC's libraries, notably base, is the purview of the Core Libraries Committee. Distinguishing between the public API of base and the internals (of base, but also ghc-prim) would help when figuring out whether we need a CLC decision when making a change.

  • I strongly feel that we should not say "ghc-prim is unstable and base is stable", at least with our current architecture. mapFB is just one example. But see the next section.

Releasing base independently from GHC

We have often talked about un-tangling base from GHC, so that a library X could depend on a single version of base, and that version of base (and the library X) could be built against multiple versions of GHC. Indeed this is precisely what base-compat is, and there is lots of prior thinking about it, e.g. Decoupling base and ghc and GHC #21800

So, I am no expert. But could we imagine this?

  • Make base-compat part of what we ship with GHC
  • Rename base to ghc-base
  • Rename base-compat to base

Then ghc-base is entirely "internals" and the entire API of base is stable. And the new base can be upgraded (or rolled back) independently of GHC. (Provided, of course, the version you pick has been fixed to work with the version of GHC you pick.)

That would be a natural fit with the thinking in this proposal.

@simonpj
Copy link
Contributor

simonpj commented Jul 26, 2022

So at first glance this does look like a reincarnation of -fglasgow-exts.

I don't think so. -fglagow-exts was just a name for a bunch of extensions, like -XHaskell2020. The motivation and mechanism for this proposal is quite different.

@adamgundry
Copy link
Contributor

Why not make this a warning -Wunstable, rather than a language extension, since presumably enabling it won't actually change the accepted language? We could even default to -Werror=unstable but users could choose to switch to -Wwarn=unstable or silence it entirely.

Possibly we could even have -Wunstable-imports and -Wunstable-language-features as separate flags? The latter seems like it may be tricky to define uncontroversially (because different people have different opinions on the merits of language extensions and the desirability of changes to them).

Personally I don't think there is a huge amount of value in -XStable.

I do think it would be nice if users could designate particular modules as unstable for importing in other packages. It's fairly common for libraries to expose their internals without making stability guarantees about them.

Co-authored-by: tomjaguarpaw <tom-github.com@jaguarpaw.co.uk>
@goldfirere
Copy link
Contributor Author

My motivation for this proposal is as stated in the proposal. That is, I don't see -XUnstable as a way to guard against internal aspect of base. For me, it is purely about unstable language features. If we wish to guard internals of base, I think we can just follow standard convention in Haskell and move some exports to e.g. GHC.Base.Internal. This would be a matter for the CLC. Furthermore, I frankly don't see a relationship between this proposal and decoupling base. I would like to decouple base, but I think that's a separate matter.

On the other hand, changing -XUnstable to e.g. -Wwarn=unstable might indeed be an improvement to this proposal. We might imagine making a new warning bucket called unstable and have individual flags for the individual features therein. Critically, we should be able to change the contents of this bucket without migration periods or other formalities.

@phadej

Proposal process was set up to bring transparency to GHC development.

Yes, and also a way for the general Haskelling public to contribute to GHC's direction. But what, precisely, is subject to the transparency policy? I don't think we want a process where a proposal is written for every refactoring. This suggests that some changes to GHC are too far from public view for a proposal. What about error messages? These have generally been deemed out of scope, but yet are very important to user experience. What this proposal does is to declare that some changes -- even if users might notice them -- are considered to be internal changes. This could be abused, of course, by declaring everything unstable and then emancipating us from this proposal process. However, we could have similar abuses today, by downgrading the quality of errors without public comment. Yet we don't do this. My hope would be that large features, such as linear types, would indeed continue to go through the proposal process, because this is helpful for making a great GHC. Put another way: I want to take things that right now require proposals and turn some cases into judgment calls. As one important example, #518 asks the committee to decide on an internal matter that few people are informed about other than @simonpj and me. The committee is not really offering informed consent, but rather rubber-stamping an issue that just happens to affect the language. Yes, the public debate is helpful in refining the design, but the decision isn't really an independent one. Even with -XUnstable, Simon and I might choose to make a proposal to seek a better design, but the committee wouldn't necessarily get the final decision.

What will prevent a feature being unstable for years? I.e. be in eternal "beta". Yet, it would be de-facto stable?
OTOH, AFAICT people knew that -XImpredicativeTypes are broken, and used it on their own risk.
Similarly people use LinearTypes or OverloadedRecordUpdate on own risk, will requiring them to also specify -XUnstable add any value?

Nothing prevents something from being unstable for years, although that's something to be avoided. If it happens, someone could always write a proposal saying it should become stable.

As for the other examples, only users who know that these features are unstable are using it knowledgeably at their own risk. A user who comes across LinearTypes or OverloadedRecordUpdate on a blog post might miss the disclaimer and thus not know that the ice is thinner there. -XUnstable serves as a way for these users to become informed about the status of these extensions. (To be clear, this proposal does not mark any extensions as unstable. Once we have the mechanism, then we could so about attaching it to various proposals.)

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jul 27, 2022

@simonpj

Can you give specific links to relevant documentation? Thanks!

Gladly! https://rustc-dev-guide.rust-lang.org/stability.html

So at first glance this does look like a reincarnation of -fglasgow-exts.

I don't think so. -fglagow-exts was just a name for a bunch of extensions, like -XHaskell2020. The motivation and mechanism for this proposal is quite different.

Sorry, in my original comment I was contrasting "at first glance" with "the actual contents. I agree it is not, in fact, the return of -fglasgow-exts.

I will say though that the Rust thing does force one to categorize why things are unstable, and I think that is good practice. Making an instability category that merely bundles other categories helps deal with coordinating concurrent migration cycles, where different things may end up becoming stable at different rates.

Also love the shout-out to splitting base!

@goldfirere
Copy link
Contributor Author

Thanks for the link to the Rust approach. I agree that having multiple different bins is likely a good design, a nudge toward the warning approach instead of the language-extension approach.

However, the Rust page describes a way for anyone to mark features as unstable. That's not part of this proposal, which is just about introducing a fixed, non-extensible mechanism, though I agree that having a way to extend it would be good.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jul 27, 2022

@goldfirere The Rust mechanism looks like it would be for anyone but actually it isn't; it only works properly in standard-library crates (crates = libraries here) :)

@phadej
Copy link
Contributor

phadej commented Jul 28, 2022

@goldfirere

As one important example, #518 asks the committee to decide on an internal matter that few people are informed about other than @simonpj and me.

But this proposal won't make #518 into an internal judgment call, or is there something which could be marked unstable, and thus make Type vs Constraint not require a proposal?

I politely disagree that it's an internal change. The addition of ==> is pretty much into users' face. (AFAICT, SORT could be indeed unstable, but ==> cannot be, can it?). So I don't see how you would avoid a proposal for #518, with or without the -XUnstable.

Copy link
Contributor

@nomeata nomeata left a comment

Choose a reason for hiding this comment

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

TL;DR of my first thoughts: This may better be a warning than a pragma, and maybe better only applies to language feature (e.g. other extensions) rather than specific imports. If it applies to imports, there ought to be a developer-accessible mechanism to mark certain modules or identifiers as unstable.

Comment on lines +31 to +36
This proposal also includes a separate extension, ``-XStable``, which prevents
``-XUnstable`` from being used. A project's ``.cabal`` file might then specify
``-XStable`` to prevent any of the project's modules from using ``-XUnstable``.
There is no ``-XNoStable``, unlike most other extensions. This ``-XStable`` component
is optional and may be removed from this proposal without affecting the
utility of ``-XUnstable``.
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d leave this out. So far we have no precedence for “.cabal file gets to set boundaries for haskell files”, and it assumes a division of roles with different “privileges” between those editing the .cabal file and those editing source files that I am not sure is realistic.

Using -XUnstable is just one of many many things a local programming policy might want to forbid (such as other extensions, unsafe features, use of emoji keywords…), and is better served by a custom hlint database maybe.

Comment on lines +51 to +53
* For years, we GHC developers knew that ``-XImpredicativeTypes`` was more of
a bug than a feature. It had no specification, and any code that type-checked
because of it was simply lucky. The feature was in essence unstable. Since
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we know that even already when -XImpredicativeTypes was fist introduced, or was that hindsight? This only counts as motivation in the former case, does it?

Comment on lines +124 to +126
#. (Technically beyond the scope of this proposal process) Tooling that helps
users automatically insert extensions (i.e. HLS) would not automatically enable
``-XUnstable`` from this message; users would have to add it manually.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is itabout -XUnstable that makes it deserve that treatment? Are there other extensions that should not be “easily enabled”? (Maybe Trustworthy, IncoherentInstances)?

And is it even good style? If the user already added ghc-prim to their cabal file, and added the imports, they are very likely determined to use ghc-prim and hopefully know why. Why make them do manual busy work that the tool could do for them?

(Yes, it’s not technically part of the proposal and the HLS authors might just choose to ignore it, or implement it by allowing automatic after a warning.)

Comment on lines +296 to +299
1. Instead of making a new language extension, we could imagine a new warning ``-Wunstable-features``.
This warning would be werror-by-default (no warnings are like this today) but could be disabled
with ``-Wno-unstable-features``. Similarly, ``-XStable`` could become something like ``-fstable``,
but this seems harder to specify in a ``.cabal`` file.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interesting point. The nix tool guard additional subcommand (a bit like extensions) under an “enable experimental features” configuration option.

So using a flag -fenable-experimental-features or -Wno-unstable-features is an alternative worth discussing.

I don’t think it’s harder to specify this in the .cabal file, ghc-options: -Wno-unstable-features`. People already know how to set warning flags in the cabal file. And those who don’t know are certainly not the target audience for this flag.

@nomeata
Copy link
Contributor

nomeata commented Jul 29, 2022

Ah, I see the discussion covers a bit of that already.

Simon writes about “Distinguishing between "internals" and "public API"” and then talks about GHC in particular, but there isn't really anything GHC specific in the desire to distinguish internal APIs and public APIs, and the issues arises in many normal libraries as well. So a solution available to “mortal” library authors would probably be better.

@monoidal
Copy link
Contributor

monoidal commented Aug 2, 2022

The list of identifiers exported from ghc-prim but not base has extraneous items. I believe only the following are correct:

GHC.Debug.debugLn
GHC.Debug.debugErrLn
GHC.Prim.Panic.absentSumFieldError
GHC.Prim.Panic.panicError
GHC.Prim.Panic.absentError
GHC.Prim.Exception.raiseOverflow   -- other similar functions are exported by "ghc-bignum".GHC.Num.Primitives
GHC.Tuple.getSolo                  -- other similar definitions are exported by "base".GHC.Tuple

The remaining functions are exported either by GHC.Exts or GHC.Base.

@goldfirere
Copy link
Contributor Author

Thanks, @monoidal. Hoogle does not report all of those re-exports from GHC.Base, which may have thrown me off the scent. (e.g. IP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants