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

Deprecate partial functions from the Prelude, giving them new quarantined homes #70

Closed
Ericson2314 opened this issue May 31, 2022 · 67 comments
Labels
awaits-ghc-proposal Blocked on an unimplemented GHC proposal (https://github.com/ghc-proposals/ghc-proposals) dormant Hibernated by proposer or committee

Comments

@Ericson2314
Copy link
Contributor

Ericson2314 commented May 31, 2022

Problem

Partial functions like head are too easy to use. The purpose of the Prelude is to guide the user, to make certain things very easy in order to encourage Haskell to be written in a certain way. The problem is that the partial functions are bad, as is widely agreed. It is unclear if there are any circumstances in which as they ought to be used, and yet Prelude makes them extremely easy to use at present, especially by new users that don't know what pattern matching is.

Many would-be solutions have been floated in the past, but I dislike them as tepid half-measures that skirt around the problem rather than confront it head on. The problem is that these partial functions are too readily available; the solution must be to make them less readily available. The solution is to make using those in the Prelude automatically raise red flags, and ideally to someday remove them from the Prelude altogether.

Solution

We want to move these functions with a very long deprecation cycle.

Prerequisite

https://github.com/ghc-proposals/ghc-proposals/blob/master/proposals/0134-deprecating-exports-proposal.rst must be implemented first.

Steps

1. Relocate functions, with reexports

This we can do immediately!

  • Create Partial modules and move functions there:

    • Data.List.head -> Data.List.Partial.head
    • Data.List.tail -> Data.List.Partial.tail
    • Data.Maybe.fromJust -> Data.Maybe.Partial.fromJust
    • ...TODO fill out more
  • Create non-deprecated reexports (since we lack the ability to do anything better) in their current locations (Data.List, Data.Maybe, etc.).

The purpose of this is allowing one to switch sooner once the deprecation mechanism is ready without running afoul of the 3 release cycle.

2. Deprecate Reexports

This we can do once the GHC proposal is implemented.

  • Deprecate the reexports of these function in their Prelude reexports those and thus inherits the deprecation.

3. (Non-normative) After many releases, and good tooling, consider removing reexports

We're not sure what is needed to do this, so just including as an aspirational goal / food for thought.
At the very least, this could only happen after:

  • 3 release policy is met, so we must have both 3 past GHCs and bases which have these functions available from the the Partial modules. (probably would be far more than 3 releases.)

  • Tooling exists to automatically perform migration guide

  • Perhaps also some sort of "base shim" so code explicitly depending on the old standard library (as opposed to say Haskell just dropped in GHCi without metadata) never stops working.

It is important to remember that not everyone that signs up to publish open source wants to be stuck maintaining it for life. Some people want to make a thing, release it, and wash their hands of it ---- think e.g. artists aren't expected to accept "patches" against a finished painting! Any hard breakage, no matter the migration tooling around it is going to to result in people pestering those once and done authors who will rightly ask "why is stuff in the language report for crying out loud changing out from under on me?"

For that use-case, only something like the base-shim that requires no changes in existing code --- essentially making this not a breakage change but just changing the defaults for new projects --- is acceptable. Indeed a social contract that a) we can and will retroactively add more and more warnings to old standards b) we will never change the meaning of old standards, is probably a good way to navigate these competing concerns in general.

Migration Guide

Merely import and ...Migration module that provides an in-use function gotten from a legacy deprecated location.

Note these properties:

  • Qualification is not necessary. One may choose to do e.g. import qualified Data.List.Partial as Partial, but import Data.List.Partial is fine.
  • Manual Prelude import is not necessary. One may choose to do e.g. import Prelude hiding (head), but this line can be omitted entirely.
  • Import lists are not necessary. One may choose to do e.g. import Data.List.Partial (head), but import Data.List.Partial is fine.

These properties follow from the nice benefits deprecated reexports provide, benefits we do not have with today's means of doing a deprecation cycle.

@tomjaguarpaw
Copy link
Member

Strongly in favour of attempting to build community consensus around this.

@Profpatsch
Copy link

Very much in favor of only doing breaking changes to base with a migration tool. We should have the technology!

@andreasabel
Copy link
Member

Concerning partial functions likehead, tail, !!, length etc. on lists, they stem from the conflation of three type-theoretically different concepts:

  1. finite lists (List a)
  2. possibly infinite lists (CoList a)
  3. streams (Stream a) which are infinite sequences

These three are lumped together in a single type [a].

  • Laziness of (:) leads to the conflation of lists and co-lists, making e.g., length a partial function.
  • The conflation of streams with (co-)lists is likely historical and for convenience (and maybe to write beautiful papers about clever value-recursive programs). But it makes head, tail, !! etc. partial which are total functions on streams.

I think a systematic fix of the problem should introduce a Stream type into base where head, tail etc. can be total. Streams would be the target of functions like iterate and repeat.
Just taking away head and tail would be very inconvenient for those that mean streams when they say [a], because head :: Stream a -> Maybe a is just utter nonsense (albeit in a different way than head :: List a -> a).

@konsumlamm
Copy link
Contributor

While I like the idea, I'm not sure it makes sense to create a Data.Maybe.Partial module just for a single function, Data,Maybe already is very small.

@soupi
Copy link

soupi commented May 31, 2022

Another thing that should be taken into account in this proposal - many learning resources introduce head and tail.

@googleson78
Copy link
Contributor

I strongly agree with what @andreasabel said - we should have, long-term, separate types for at least a finite list and streams. I think that that effort is however much larger and that this proposal might even be a step in that direction and/or part of the process (once we've cleaved the functions that are valid on Streams into a separate place it will be much easier to push people to use the total versions that operate on Streams), and additionally, this proposal by itself has great value, so I would like to see this happen first.

@Gabriella439
Copy link

Gabriella439 commented May 31, 2022

I don't believe that adding other list-like types to Prelude should be a precondition for removing the partial versions from base

My reasoning is that if the Prelude did not already have these partial functions, I'm fairly confident the committee would not accept adding such partial functions if they were proposed today

@Ericson2314
Copy link
Contributor Author

@andreasabel Yes I have wanted Stream at least since using https://hackage.haskell.org/package/clash-prelude-1.6.3/docs/Clash-Signal.html, but I agree with @Gabriella439 that this shouldn't be blocked on having Stream exist. Besides the reason she said, here are a few more:

  1. Even if we had Stream we would still need Data.List.Partial to have the smooth migration. I don't think an auto-migration tool could deal with the changing types, let alone decide whether one actually wanted Stream or NonEmpty since they both e.g. make head and tail safe.

  2. The design here is fully compatible with adding Stream later. (Right?)

  3. I want to establish precedent that migration tools alone can "unstuck" us with old controversies. We have many other situations where "nice situation to new learners" and "less breakage for old time users to fix" are in conflict, and I would like every time to use auto-migration tools to avoid a tradeoff and have our cake and eat it too.

That last one, while more of a meta concern, is most important to me. I think in the past we've done a bad job at understanding extent to which are conflicting priorities, and thus haven't been able to muster the energy to do things which would actually defuse the conflict (better migration tools!). I am really hoping we can turn a new leaf.

@andreasabel
Copy link
Member

2. The design here is fully compatible with adding Stream later. (Right?)

Indeed so!

@goldfirere
Copy link
Contributor

A subtlety in @andreasabel's point is that what counts as partial depends on your viewpoint. If you primarily think of [a] as a stream, then head is total and find is partial (even though it returns a Maybe). So I think we're really saying here: label functions as partial if they can fail on finite data. Or (perhaps equivalently?), label functions as partial if they can throw an exception, but pretend that potentially non-terminating functions are total. To be clear, I'm in support of this viewpoint -- I don't want to have to go through a Partial module to get length! -- but it is an opinion that we're establishing.

I'd also like to add a small spin to the requirements for the migration tool: that it can be called as a source plugin. That is, I want to be able to write -fplugin=Migration.ImportPartials (or whatever) on the command line, in a cabal.project or in an OPTIONS_GHC to perform a transformation to my source that will allow it to compile with e.g. import Data.List (head) in perpetuity. This is particularly important to authors, who can now post the incantation on their errata page, instead of providing detailed instructions for how to repair all of the examples in their book.

Happily, this requirement should not be hard to accommodate, because any migration tool essentially has to do all of this work already. I'm just specifying a particular interface. Note also that GHC already has support for code transformations that are intended to preserve comments, indentation structure, etc., so phrasing the migration tool as a plugin might even simplify the task.

@Ericson2314
Copy link
Contributor Author

Ericson2314 commented Jun 2, 2022

So I think we're really saying here: label functions as partial if they can fail on finite data.

Well to be really pendantic, it's fail in finite steps.

data X = Foo X | Bar X
foos (Foo x) = () : foos x

bad = let x = Bar x; in foos x

here foos has no finite data to fail on, but it is failing in finite steps.

I bring this up basically to say that Haskell never tries to save you from non-termination, but should try to say you from what we were calling "incompleteness" in new incomplete (missing patterns in case, missing fields in constructor, etc.)

The good thing about this is that it is an objective principle, not mattering on ones interpretation of what is [a] is for: Even if [a] is idiomatically infinite, trying to get the user to write correct co-terminating functions is out of scope, however failing to handle [] (the empty list) where it allowed to occur is always dubious style.

@Ericson2314
Copy link
Contributor Author

Ericson2314 commented Jun 2, 2022

-fplugin=Migration.ImportPartials

My preferred stretch goal would instead be shim libraries: if the user includes a build-depends base < 4.19, and we can make a base 4.19.x that depends on the newer base, the longevity story is even better! No errata is needed: cabal just downloads the shim and everything works.

Still this reminded me it's good to think in terms of what is blocking each step. I split step 1 into 1 and 2 to better leverage this:

  1. The new home can be done immediately.
  2. The deprecation can be done once we have deprecated reexports.
  3. The final removal can happen after whatever other migration tools we want at also ready.

This means, sure, plugin and shim base why not! What I am really concerned with is getting to steps 1 and 2. If 3 doesn't happen for years and years so be it. At the same time, I think getting to step 2 will make this process "feel real", and the psychological boost of finally dealing with this longstanding bugbear will inspire us to write whatever migration tools we deem are necessary.

@goldfirere
Copy link
Contributor

I agree with what you just wrote @Ericson2314 -- in particular, about having step 3 be far in the future.

So maybe the current proposal should just have steps 1 and 2, keeping step 3 aspirational. That is, we commit to

  1. Making a new home for these functions
  2. Deprecating exports once that is possible

but that's all we commit to. In general terms, the proposal can also say that we would like to actually remove the deprecated bits in the future, but that depends on a migration tool, or plugin, or reinstallable base, or some other mitigation strategy. Because we don't know yet what mitigations will actually be possible (that is, actually implemented -- they're all possible), we don't commit to any one. Furthermore, it would take a new proposal to hash out and agree to the specifics.

I have to admit that removing a concrete plan for (3) makes me considerably more enthusiastic about this proposal. I'm scared of (3), precisely because we don't exactly know what (3) looks like yet.

@Ericson2314
Copy link
Contributor Author

@goldfirere Sounds great to me! Increasing enthusian at the mere cost of making (3) a bit more nebulous is a phenomenal trade as far as I am concerned :).

@Ericson2314 Ericson2314 changed the title With deprecation cycle, remove partial functions from the Prelude Deprecate partial functions from the Prelude, giving them new quarantined homes Jun 2, 2022
@Ericson2314
Copy link
Contributor Author

@goldfirere I revised and retitled this; do let me know if it addresses those concerns or more is needed.

@Gabriella439
Copy link

Gabriella439 commented Jun 2, 2022

I don't know if this is in scope for this proposal or not, but I wanted to ask if there was a plan for the new total replacements. I can see two main options, but maybe there are more:

  • Create new total functions in the Prelude that replace the old partial ones but return a Maybe value
  • Don't replace them and instead direct people to use the equivalent total functions on NonEmpty lists

My personal preference is the latter option, for what it's worth

@Ericson2314
Copy link
Contributor Author

Ericson2314 commented Jun 2, 2022

I don't think they are in scope or out of scope a priori, but just like with Stream, I would rather propose as few things as I can get away with. E.g. Demoting (3) to be a non-normative part feels great because it makes the proposal smaller.

(FWIW, I think the primary replacement for these functions is arguable just good ole' pattern matching, and the situation we most want to avoid is people using partial functions because they never pattern matching. For more advanced users I agree fmap head . nonEmpty and similar is very nifty and elegant, and precludes needing more list functions with any urgency.)

@david-christiansen
Copy link

The "...TODO fill out more" is doing an awful lot of work in this text. How many actual names are you proposing to remove? There's a big difference between 3 and 30, after all, and that can have an impact on where they are moved to.

Now, with my stodgy conservative hat on: do we have a sense of the practical impact of these functions for actual developers? Have any of us heard of serious problems caused by the unjustified use of fromJust? It seems plausible to me that, while the aesthetic benefit of removing these functions is real, the practical benefit may actually be extremely small. At the same time, the cost might be quite large - even if migration is very easy, and supported by tools, developers still need to dust off their machinery, make some time, remember how to build their old project, run the tool, remember our Hackage login, and upload it - for those who do this regularly, it's no big deal, but if I wrote some random library in 2015 and someone is bugging me to release a new version, then there might be hours of overhead on a simple, easy, tool-supported migration. Furthermore, the people who will bear this burden will never, ever show up in these threads to comment.

Here's an alternative proposal sketch that I think achieves your goal of "to guide the user, to make certain things very easy in order to encourage Haskell to be written in a certain way", but with less potential disruption. What if warnings are emitted for a set of known historical warts, but only when the language chosen is GHC20XX instead of Haskell2010? And that's it?

@Ericson2314
Copy link
Contributor Author

Have any of us heard of serious problems caused by the unjustified use of fromJust?

Yes, in the days before they were given HasCallStack, fromJust used to have exceptionally bad error-tracking for bootstrapping reasons. We once had a fromJust in dependency code ruining a mobile app (already harder to debug) that took 1 or 2 days to track down. Even with HasCallStack these are still lurking foot-guns that could ruin the experience of users of your software --- it isn't like they enjoy debuggin your app for you regardless of how good the tracing info is!

What if warnings are emitted for a set of known historical warts, but only when the language chosen is GHC20XX instead of Haskell2010? And that's it?

A key part of the above story was that the fromJust was in dependency code. The biggest threat here isn't the code you wrote,if you trust yourself, it's the code other people wrote that you never had time to audit. If dependency libraries escape diagnostics just by using Haskell2010, well, that completely defeats the intended benefit being able to audit other code, and goad other authors into doing the right thing in the first place.

To put this in a positive light, I want to be able to trust other programmers I've never met as much as possible. To me, the very field of programming languages is first and foremost about facilitating large scale human collaboration, and enabling more trust is a key part of that.

At the same time, the cost might be quite large - even if migration is very easy, and supported by tools, developers still need to dust off their machinery, make some time, remember how to build their old project, run the tool, remember our Hackage login, and upload it - for those who do this regularly, it's no big deal, but if I wrote some random library in 2015 and someone is bugging me to release a new version, then there might be hours of overhead on a simple, easy, tool-supported migration. Furthermore, the people who will bear this burden will never, ever show up in these threads to comment.

With all do respect, I think you are vastly overestimating the costs here. Since 3 is non-normative and deferred indefinitely, are we only proposing a software deprecation, which is a warning. No one needs to fix anything.

if there were build failures, sure, I bet these developers of old libraries would be badgered just as you say. And that would be annoying. But I just don't think people take the time to write those emails for plain warnings --- the type of frustration that leads to such an email just isn't caused by anything less than a build-failure. Anyone that does care about warnings in other code I (perhaps idealistically :) ) believe is also sufficiently not in the rush to send a PR / patch rather than complaint, and that is a lot less burdensome for the upstream developer.

Now, someone that got bit by a fromJust failure, rather than just saw a warning go by in the build logs, is indeed very frustrated! But they aren't going to wait for the upstream dev fix their broken program; they're either going to stop the new library or patch it themselves right away. If they patch it themselves, maybe the commit message will be angry, but at least the upstream dev is receiving a PR / patch still.

@david-christiansen
Copy link

I find your argument for the value of giving warnings to more people convincing - thanks! Esp. without the actual deletion.

@Ericson2314
Copy link
Contributor Author

Ericson2314 commented Jun 3, 2022

Thanks! I will in turn add the downsides you mention to (non-normative) step 3 as thing migration tooling indeed cannot fix. edit done.

@ketzacoatl
Copy link

I am strongly in favor of this effort and am really excited to see a lot of really great discussion to refine the proposal towards something we can take action on!

@soupi
Copy link

soupi commented Jun 3, 2022

do we have a sense of the practical impact of these functions for actual developers? Have any of us heard of serious problems caused by the unjustified use of fromJust? It seems plausible to me that, while the aesthetic benefit of removing these functions is real, the practical benefit may actually be extremely small.

Yes, this is a real problem. I'm tired of having to mentor juniors around these issues, writing style guides / best practice guides with dedicated sections to partial functions, and having to be defensive when reviewing code. While they are fine for hobby projects and one-off scripts, production code should not use partial functions because they make the code crash.

@runeksvendsen
Copy link

3. (Non-normative) After many releases, and good tooling, consider removing reexports

We're not sure what is needed to do this, so just including as an aspirational goal / food for thought. At the very least, this could only happen after:

  • 3 release policy is met, so we must have both 3 past GHCs and bases which have these functions available from the the Partial modules. (probably would be far more than 3 releases.)
  • Tooling exists to automatically perform migration guide
  • Perhaps also some sort of "base shim" so code explicitly depending on the old standard library (as opposed to say Haskell just dropped in GHCi without metadata) never stops working.

(my emphasis)

I think the proposal needs to clarify what constitutes a "migration". If we remove, say, fromJust from base, then one valid migration — ie. a change that fixes compilation of a library that uses fromJust — would be to simply inline the old definition in the user's code, ie.:

import Data.Maybe (fromMaybe)
fromJust = fromMaybe (error "Maybe.fromJust: Nothing")

which, of course, would achieve nothing at all.
Therefore I think the proposal should specify what constitutes a proper migration, in order to avoid just moving code from base and into the user's code base.

do we have a sense of the practical impact of these functions for actual developers? Have any of us heard of serious problems caused by the unjustified use of fromJust? It seems plausible to me that, while the aesthetic benefit of removing these functions is real, the practical benefit may actually be extremely small.

Yes, this is a real problem. I'm tired of having to mentor juniors around these issues, writing style guides / best practice guides with dedicated sections to partial functions, and having to be defensive when reviewing code. While they are fine for hobby projects and one-off scripts, production code should not use partial functions because they make the code crash.

I think this is mixing things together. Are you under the impression that removing partial functions from base can replace mentoring junior developers and/or spending less energy reviewing code when the function we're proposing to remove from base can just be inlined? The same goes for whether or not production code should use partial functions: this is a separate discussion, and it's not at all clear whether removing the suggested partial functions from base will have any an effect on this.

Usually, the way things go are:

  1. Write an application for a customer that does thing X, being careful to avoid partial functions
  2. Customer asks to add feature Y, which requires a complete refactor in order to avoid partial functions
  3. Make a decision: spend the time to refactor the code to avoid the use of partial functions (and thus postpone delivery of the new feature) or don't refactor and live with the partial functions (while enabling faster delivery)

This sort of cost-benefit analysis is non-trivial, and which functions are present in base will not have an effect on the decision made (since the effort of inlining is so small).

@Ericson2314
Copy link
Contributor Author

Ericson2314 commented Jun 5, 2022

@runeksvendsen

Step 3 is non-normative so I wouldn't spend too much time thinking about it (and maybe I should just remove it) but the migration guide I was thinking of was merely importing the needed function from Data.List.Partial or Data.Maybe.Partial. That is even more minimal and perhaps "achieving nothing" than what you had in mind.

That's OK to me however: the migration isn't supposed to make existing code better, it's just supposed to merely keep it working and I suppose also not make it worse (as inlining functions might).


Are you under the impression that removing partial functions from base can replace mentoring junior developers and/or spending less energy reviewing code when the function we're proposing to remove from base can just be inlined?

I don't think anyone is arguing that we can skip teaching. The argument specifically was about new users grabbing head and tail because the were so convenient, before they learned what pattern matching is. That using head and tail are so easy and available could interrupt learning that would otherwise happen. So yes teachers should remain vigilant, but that doesn't mean we shouldn't try to use footguns from the curriculum.

Make a decision: spend the time to refactor the code to avoid the use of partial functions (and thus postpone delivery of the new feature) or don't refactor and live with the partial functions (while enabling faster delivery).

That's a tough question, but I don't think that's the question were trying to answer with this proposal! For me personally and all the projects I have worked the precept is, partially may be the answer but head, tail, and fromJust never are. The reason is simple: business-specific partiality must be implemented with business-specific code.

Under that precept,

head bussinessItems

is never acceptable.

case bussinessItems of
  x : _ -> x
  [] -> error "the function formerly known as Data.List.head"

is also never acceptable.

case bussinessItems of
  x : _ -> x
  [] -> error "no business items. Because ...business reason... this should never happen."

might be acceptable.

head is slightly matter to write, but as no global refactor is being forced the benefits of writing the error ... (calling attention in to in the code for the next reader, being moderately bothersome to remind the programmer this should never be the first choice, etc.)

Now, I wouldn't want to impose this rule on the Haskell community at large, but having to write import Data.Foo.Partial to get access to the things (without a deprecation warning) should both be not too burdensome for programmers that really want to use them, and a smaller reminder to programmers that they are doing a bad thing in contexts where they really indeed shouldn't be used.

@runeksvendsen
Copy link

Thank you for clarifying, @Ericson2314. I had understood Step 3 to mean "removing head/fromJust etc." entirely from base. After re-reading, though, I can see that this was a misunderstanding on my part.

I agree completely regarding using fromMaybe (error "BUG: <function name>: expected non-empty list") . listToMaybe instead of head in order to communicate which assumption is broken in case of an empty list (or a Nothing value in case of fromJust), while simultaneously making the error-call easy to locate in the code.

I don't think anyone is arguing that we can skip teaching. The argument specifically was about new users grabbing head and tail because the were so convenient, before they learned what pattern matching is. That using head and tail are so easy and available could interrupt learning that would otherwise happen. So yes teachers should remain vigilant, but that doesn't mean we shouldn't try to use footguns from the curriculum.

To be clear: I also agree that discouraging the use of partial functions is valuable. However, this value must be weighed against the cost of being a breaking change.
In these discussions about breaking changes, it's my impression that the only disagreement is which cost to assign to breaking changes. In other words: we all agree that there's value in the change, but we don't agree on whether that value outweighs the cost of introducing a breaking change because there's a disagreement on exactly how great the negative impact is.

@Ericson2314
Copy link
Contributor Author

Sounds good and glad to hear it, @runeksvendsen. Without step 3 there is no breaking change, just warnings, so I hope we're successfully punting on that one lingering issue.

@AntC2
Copy link

AntC2 commented Jun 11, 2022

many learning resources introduce head and tail

For example LYAH intro level talks about head, tail, last. It does warn these are partial.

Are you under the impression that removing partial functions from base can replace mentoring junior developers ...?

These functions have been part of Haskell since ... aww 1990. And part of LISP since 1960's. I haven't noticed the sky falling yet.

[bad things] too easy to use
... because they make the code crash.

If that's the objective, isn't divide by zero or Int over/underflow too easy to use? (Oh, I suppose overflow doesn't cause a crash, it just delivers a nonsense result.) No amount of tinkering with long-standing parts of the infrastructure will remove all "footguns"/there's plenty of ways to make code crash. Indeed I'm not sure I'd call these specifically footguns: newbs find out about them as readily as the dangers of subscript out of array. They gotta learn to think about what they're doing some time.

These functions are perfectly safe if you've first checked the list is not null. (And there's plenty of ways of getting that, in effect.) Then that sounds like something to build into a linter, or Liquid Haskell.

Really, doesn't everybody have better things to do?

Do we want Haskell to turn into a bondage and discipline language?

@Bodigrim
Copy link
Collaborator

But why would we like to preserve partial functions undeprecated in Data.List.Partial?

@hasufell
Copy link
Member

hasufell commented Jun 24, 2022

I don't have a too strong opinion about moving this stuff to another module, but I do like partial functions in some circumstances and I see no problem with using them correctly.

foo :: String -> String
foo str
  | null str = str
  | last str == ' ' = "lol"
  | otherwise = "shrug"

Yes, I can jump through hoops and redefine that function with unsnoc, but what's the point. This function is total.

If base Prelude at some point removes them, I'll simply write my own Prelude that redefines them.

I also don't see how this fixes the larger problem of "I want to know that my program doesn't crash". Linters can already help with the obvious partial functions, projects have ways to enforce these things to some degree (e.g. haskell/haskell-language-server#2974).

Increasing visibility about partial functions is fine: documentation, HasCallStack, modules named Foo.Unsafe (I personally prefer if the functions start with unsafe, such as unsafeHead). But let's not remove them.

@chessai
Copy link
Member

chessai commented Jun 24, 2022

I don't have a too strong opinion about moving this stuff to another module, but I do like partial functions in some circumstances and I see no problem with using them correctly.

foo :: String -> String
foo str
  | null str = str
  | last str == ' ' = "lol"
  | otherwise = "shrug"

I don't think foo is a particularly good example. With a total version of last:

last :: [a] -> Maybe a
last [x] = Just x
last (_ : xs) = last xs
last [] = Nothing

foo :: String -> String
foo str = case last str of
  Just ' ' -> "lol"
  _ -> "shrug"

This version of foo is fewer lines and doesn't require a boolean check (null str), which, when omitted, would still compile.

@hasufell
Copy link
Member

@chessai yeah, I'm not going to dig up all the examples of total use of partial functions, because there are infinite.

@Bodigrim
Copy link
Collaborator

I think the conservative solution to add {-# WARNING #-} is a reasonable option. We should not use {-# DEPRECATED #-} unless there is a specific intention and schedule to remove in a finite amount of time, but {-# WARNING #-} does not break anyone and just states a simple fact about head / tail.

@sullyj3
Copy link

sullyj3 commented Jun 25, 2022

But why would we like to preserve partial functions undeprecated in Data.List.Partial?

It's nice to have escape hatches. Sometimes a quick and dirty hack is just more convenient, maybe for short lived code or learning projects. We should discourage patterns that have a tendency to bite you in the ass, without assuming that we always and everywhere know better than the programmer. The optimal level of paternalism is some, rather than lots.

I've used Elm, a language that disallows partial functions almost entirely, and it can be a very frustrating experience.

@hasufell
Copy link
Member

hasufell commented Jun 25, 2022

It's nice to have escape hatches.

I wouldn't even call it escape hatch. Haskell is not a total language. Partial functions are well within the scope of the report.

Making them more distinct is a good idea. But removing them doesn't make much sense, unless you want to remove partial pattern matching as well and whatnot.

What would really be interesting is if GHC could tell me whether function foo has a partial pattern match or an explicit error call, after the entire body has been completely inlined.

@goldfirere
Copy link
Contributor

One real drawback to the WARNING approach is that it's hard for clients to accommodate. That is, suppose I want to use head, for reasons that are my own. If there's a WARNING, then I have to disable the warning. But disabling user-written warnings is too broad: I'd disable warnings that others have written and that may be relevant to me. A deprecated (or warned) re-export handles this nicely. It means that I get the diagnostic by default, but I can suppress it on a per-function basis by importing the un-warned version.

@Bodigrim
Copy link
Collaborator

@goldfirere that's not the case here. If there is a demand for head / tail without warnings, anyone can create a separate package providing Data.List.Compat without any warnings in under 10 minutes. I'm opposed to adding such module to base itself: such crutch is likely to ossify forever, increasing weirdness.

@Ericson2314
Copy link
Contributor Author

I started an implementation of step 1.

Part of the reason of having the step is to find what in base might depend on these functions already. Hopefully not much, or things we can get rid if. But insofar that we need to keep the definitions, I sort of soon make the module which provides them too, even though I wold tell people never to use that module.

In a future split-base world, I would indeed move the module to a separate library and just have base do a module-level deprecated reexport telling people to depend on the e.g.. partial-functions packages explicitly.

@goldfirere
Copy link
Contributor

The problem is that many people might make Data.List.Compat modules in a variety of packages, and this is bad for the ecosystem. This isn't terrible, but I just find it needlessly painful for users. Instead, I do think it's appropriate for base to have e.g. Data.List.Partial.

Another possible way forward is to use ghc-proposals/ghc-proposals#454. This would allow the possibility that all partial functions are grouped under one warning flag, so they (and only they) could be permitted warning-free. This stops the problem I mentioned in my post, where I was worried about a user accidentally suppressing a warning that they want to see. Perhaps it's too troublesome to depend on an unimplemented feature like ghc-proposals/ghc-proposals#454, but it might also be a sweet spot in the design.

@Ericson2314
Copy link
Contributor Author

I think I still like the separate module because it makes it shows up in code being read without diagnostics etc. Fancy warnings are great but to someone just perusing the code I think seeing import Data.List.Partial (head) it something nicer than {- OPTIONS_GHC -Wno-base-partial-functions #-}.

@emilypi
Copy link
Member

emilypi commented Jun 30, 2022

I agree with @Ericson2314 and @goldfirere here, though I think dedicating a warning for partiality is perhaps a step too far in pathologizing their use. Making them a deliberate inclusion into one's codebase with an import in my mind is sufficient for general use.

Slightly off-topic, but if GHC could detect partiality (either user-denoted or detected by the compiler), then I'd be in favor of a warning that tells one when there's a partial function in play. That to me seems more generally useful. Conversely, a totality checker and associated warnings would be similarly helpful.

@michaelpj
Copy link

Just to add another feather to the pile in favour: HLS gets a steady stream of bug reports from people whose entire language server goes down with no diagnostics whatsoever because it hit a partial function somewhere in HLS or its dependencies. This is a) really embarrassing, and b) really hard to track down. If it was Java and we at least got a stack trace I could live with it, but as it is I can't see it as anything other than irresponsible to allow partial functions without informative errors into your application or any of your transitive dependencies. So anything that nudges the ecosystem away from them is good by me.

@Tarmean
Copy link

Tarmean commented Jul 8, 2022

My understanding is that this proposal tries to urge all Haskell users to a unified, slightly-more-total, style. Is there a possibility for this proposal to add an equivalent to Rust's 'expect' method or would this be out of scope? For context:

let x = Some("value");
assert_eq!(x.expect("error message for None case"), "value");

This gives a unique 'this is impossible' assertion syntax without being overly verbose.
This is partially personal preference, I think explicit case statements over-emphasize which constructor may be returned, which I think is usually comparatively easy to reason about, while actively obscuring other properties such as termination or correctness.

     -- This always terminates. But the important property isn't lists vs streams, it's '[1..] contains even numbers'
    case safeHead $ filter even $ map (*3)  [1..] of
        Nothing -> error "List is infinite"
        Just out -> out

More to the point, without some syntactically cheap unwrapping operation I'm worried that people will be drawn to do-notation for error propagation which breaks HasCallStack

safeAndGoodCode :: Ord a => [a] -> M.Map a b -> Maybe b
safeAndGoodCode ks dict = do
    k <- safeHead ks
    v <- dict M.!?  k
    pure v

This has some of the fun properties of null - it completely obscures where the error comes from and is even worse than a CallStack-free 'head: Empty list'.

Also, since non-exporting is the theoretical end-goal, I'd be interested in how much the expected (and acceptable) ecosystem breakage would be. Since I still semi-regularly encounter AMP regressions my gut-feeling is >50% hackage packages being affected and >5% never being fixed could be realistic, but I have no idea what the actual numbers would be.

@hasufell
Copy link
Member

hasufell commented Jul 8, 2022

Just to add another feather to the pile in favour: HLS gets a steady stream of bug reports from people whose entire language server goes down with no diagnostics whatsoever because it hit a partial function somewhere in HLS or its dependencies. This is a) really embarrassing, and b) really hard to track down. If it was Java and we at least got a stack trace I could live with it, but as it is I can't see it as anything other than irresponsible to allow partial functions without informative errors into your application or any of your transitive dependencies. So anything that nudges the ecosystem away from them is good by me.

I'm not sure the core issue here is actually partial functions. Yes, there were a few annoying instances in HLS, e.g. this one, which I actually found pretty easy to track down though. I'm not saying this is good ergonomics. A stack trace should be in place here, I totall agree... it gets much worse if those bugs are in libraries.

The larger issue here is that any library function can just bottom out or throw exceptions, even in the absence of partial functions. This is particularly true when dealing with filesystem (which HLS does a lot... and those errors can also be pretty annoying like the infamous commitBuffer: invalid argument on windows). Here I believe it's up to the application to ensure that it doesn't crash (compare with most webservers, which don't crash on handler exceptions or even partial functions either) and employ strategies to recover from unexpected failure.

@michaelpj
Copy link

I was actually thinking of this issue: haskell/haskell-language-server#3002

The only reason we made any progress on this is because Pepe happened to be there and remember "oh, I maybe used that function in this dependency I wrote once". If we hadn't had that we'd have been pretty stumped.

Throwing exceptions from filesystem code that runs in IO is much more okay: we can and do catch those exceptions. Random failures from pure code are much more problematic.

I do agree that the problem is not partial functions per se, but rather casual use of partial functions that provide no error context. If we lived in a culture where every use of partial functions was accompanied with a careful comment explaining why it was safe, and an informative error message... that might be okay.

@hasufell
Copy link
Member

hasufell commented Jul 8, 2022

If we lived in a culture where every use of partial functions was accompanied with a careful comment explaining why it was safe, and an informative error message... that might be okay.

I think this is a much more useful goal!

@michaelpj
Copy link

I think this proposal would move us towards that world.

@sullyj3
Copy link

sullyj3 commented Jul 9, 2022

let x = Some("value");
assert_eq!(x.expect("error message for None case"), "value");

This gives a unique 'this is impossible' assertion syntax without being overly verbose.

Relude's flipped fromMaybe operator (?:) provides nice notation for this:

Just "value" ?: error "error message for Nothing case"

Maybe that could find its way into base.

@tomjaguarpaw
Copy link
Member

If we lived in a culture where every use of partial functions was accompanied with a careful comment explaining why it was safe, and an informative error message... that might be okay.

I think this is a much more useful goal!

I think this proposal would move us towards that world.

I agree. It's one thing to have a goal and quite another to have a concrete plan of steps that might help towards that goal.

@Bodigrim Bodigrim added the awaits-ghc-proposal Blocked on an unimplemented GHC proposal (https://github.com/ghc-proposals/ghc-proposals) label Nov 20, 2022
@Ericson2314
Copy link
Contributor Author

@Bodigrim I do not think this is blocked on a GHC proposal, I think this is blocked on me making a step 1 merge request for the CLC to vote on.

@Bodigrim
Copy link
Collaborator

Bodigrim commented Mar 6, 2023

@Ericson2314 the proposal says very clearly:

Prerequisite
https://github.com/ghc-proposals/ghc-proposals/blob/master/proposals/0134-deprecating-exports-proposal.rst must be implemented first.

But besides that I'd like to see how #87 and #114 turn out in the wild before trying a different approach, as it might be unneeded after all.

@Bodigrim
Copy link
Collaborator

Given that there does not seem to be any progress or activity on https://github.com/ghc-proposals/ghc-proposals/blob/master/proposals/0134-deprecating-exports-proposal.rst and other constraints, let's mark this as dormant for now. Rest assured that this is not a sign of dismissal, it's just that we strive to keep the list of open issues actionable.

@Bodigrim Bodigrim added the dormant Hibernated by proposer or committee label Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaits-ghc-proposal Blocked on an unimplemented GHC proposal (https://github.com/ghc-proposals/ghc-proposals) dormant Hibernated by proposer or committee
Projects
None yet
Development

No branches or pull requests