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

Add safe function to convert partial functions on empty lists to total functions? #69

Closed
Gabriella439 opened this issue May 29, 2022 · 55 comments
Labels
withdrawn Withdrawn by proposer

Comments

@Gabriella439
Copy link

The idea is to add this function to Data.List:

safe :: ([a] -> b) -> [a] -> Maybe b
safe f [] = Nothing
safe f xs = Just (f xs)

… which you can use like this:

safe head :: [a] -> Maybe a

safe last :: [a] -> Maybe a

safe tail :: [a] -> Maybe [a]

safe maximum :: Ord a => [a] -> Maybe a

… and so on. Another possible variation on this proposal is to define safe to work on Foldables instead of monomorphic lists:

safe :: Foldable f => (f a -> b) -> f a -> Maybe b
safe f xs
    | null xs   = Nothing
    | otherwise = Just (f xs)

… in which case it would probably go in Data.Foldable instead of Data.List.

@Bodigrim
Copy link
Collaborator

I think it's a bit misleading: safe (tail . tail) is not safe at all.

@Gabriella439
Copy link
Author

Somebody asked me on Twitter if the generated code for something like safe head would optimize away the error, and the answer is that it appears to do so, at least when the relevant functions are defined within the same module

module Example where

safe :: Foldable f => (f a -> b) -> f a -> Maybe b
safe f xs
    | null xs   = Nothing
    | otherwise = Just (f xs)

safeHead :: [a] -> Maybe a
safeHead = safe head
$ ghc -O2 -ddump-simpl -dsuppress-all Example.hs

safeHead
  = \ @ a_a1wg xs_ary ->
      case xs_ary of {
        [] -> Nothing;
        : ds1_a2oE ds2_a2oF -> Just ds1_a2oE
      }

@Gabriella439
Copy link
Author

@Bodigrim: The reason I call this function safe is because basically every non-partial base / Prelude alternative calls these functions safeHead, safeTail, etc. I'm following existing widespread naming conventions in the ecosystem

@jneira
Copy link
Member

jneira commented May 29, 2022

I think it's a bit misleading: safe (tail . tail) is not safe at all.

yep the name should be changed, not sure what could be the alternative

@thielema
Copy link

thielema commented May 29, 2022 via email

@Gabriella439
Copy link
Author

@Bodigrim @jneira: Another alternative could be to call the function "safer", to indicate that there is still the possibility that the result is partial, but it is less partial than before

@Gabriella439
Copy link
Author

@thielema: The goal is to provide something that is ergonomic. What you are proposing is significantly less ergonomic

For example, compare this:

safe head [ 1, 2 ]

… versus your proposal, which would be:

-- Assuming that you were proposing:
-- safe :: [a] -> Maybe (NonEmpty a)
fmap NonEmpty.head (safe [ 1, 2 ])

@JakobBruenker
Copy link

-- Assuming that you were proposing:
-- safe :: [a] -> Maybe (NonEmpty a)

FWIW, that function already exists, Data.List.NonEmpty.nonEmpty

@thielema
Copy link

thielema commented May 29, 2022 via email

@tomjaguarpaw
Copy link
Member

This doesn't make sense to me. I can't imagine a situation where it would be better to use safe than just to use a full solution, for example Data.List.NonEmpty, instead.

@Gabriella439
Copy link
Author

@thielema @tomjaguarpaw: The problem I'm trying to solve is that right now the path of least resistance is to use the partial functions on lists because:

  • These partial functions on lists are included in the Prelude by default (unlike their NonEmpty alternatives)
  • The corresponding total NonEmpty functions collide with the partial functions from the Prelude, so they have to be imported qualified or the partial functions have to be hidden
  • These partial functions work on ordinary lists which (for better or worse) get special syntactic treatment within the language

So I'm not approaching this problem from the standpoint of "there needs to exist a way to compute a safe head". I already know how to do that and it can be done with enough code/imports.

Rather, I'm approaching this from the standpoint of "there needs to be a way to compute a safe head that is as ergonomic as the partial head so that the safe version is now the path of least resistance"

@tomjaguarpaw
Copy link
Member

I think that exporting safe versions of all crashing functions in Prelude is better than "safe", which seems to be far too clever for its own good.

@Gabriella439
Copy link
Author

Gabriella439 commented May 29, 2022

@tomjaguarpaw: That was my original idea, but a prior proposal of mine was rejected due to the Fairbairn threshold so this was an attempt to create a proposal that was "Fairbairn threshold"-clean.

Specifically, if you have a safe function then there's no reason to define a safeHead function because of the Fairbairn threshold:

safeHead = safe head

… because (quoting the Wiki page):

In particular any method whose definition isn't much longer than its name (e.g. fooBar = foo . bar) falls below the threshold.

@JakobBruenker
Copy link

I would argue that the Fairbairn threshold isn't the only thing that matters here - it seems like a good idea to have a solution that doesn't rely on partial functions, such that you can eliminate them from your code base.

@Gabriella439
Copy link
Author

Gabriella439 commented May 29, 2022

@JakobBruenker: See my comment here: #69 (comment)

The goal here is not to propose that safe head is the blessed way to compute a safe head.

Rather the goal here is to provide an ergonomic total alternative to partial functions to stem the bleeding where people use the partial versions out of convenience.

@wygulmage
Copy link

[bikeshedding] A name like nonNull, onNotNil, nonNilWith, though a few letters longer than safe, might be more clear -- especially for a Foldable-generalized version. Is there something like this in the monofoldable or RIO hierarchy of libraries? I recall them using a nonempty newtype for foldables.

@Gabriella439
Copy link
Author

Yeah, I'm not too attached to the name. I'd be fine with alternatives that don't use the word safe if that's what it takes to get this approved

@jneira
Copy link
Member

jneira commented May 29, 2022

+1 to onNotNil, onNotNull or onNotEmpty

@tomjaguarpaw
Copy link
Member

but a prior proposal of mine was rejected due to the Fairbairn threshold

I don't really follow the logic. Firstly, safe is a small enough function that it doesn't pass the Fairbairn threshold. Secondly, the Fairbairn threshold shouldn't really come into play here, because it's a question of providing safer functionality, not more functionality. My problem with safe is it's not much safer:

  1. safe (tail . tail) is not safe (per @Bodigrim)
  2. you have to know which list functions it should be applied to, and once you know that it's not terribly useful any more, because it's the knowledge of which Prelude functions are unsafe that's important, not finding replacements for them.

So I don't think I'd like to introduce safe into Prelude before:

  1. We've documented crashing functions in Prelude with big, red warnings in the Haddock
  2. We've introduced their non-crashing alternatives into Prelude

(at which point safe probably becomes redundant).

@Ericson2314
Copy link
Contributor

I think we just need to rip off the bandaid and remove from the Prelude after a deprecation cycle.

The problem isn't that people need help writing total code, but that writing partial code is too often the path of least resistance. Having a safe function that one must remember to use doesn't help change the path of list resistance.

@Ericson2314
Copy link
Contributor

Ericson2314 commented May 29, 2022

I see above @Gabriella439 also mentioned the path of least resistance by name too too. I just don't believe adding more things to Prelude can help. head is easier than safe head. head is in fact one of the easiest things period — a plain function exported from Prelude is at the front of the Huffman priority queue. I think it's doomed to try to make something else more easy, all we can do is make these partial functions less easy than they are today.

@Gabriella439
Copy link
Author

Gabriella439 commented May 29, 2022

@tomjaguarpaw: I genuinely do not understand the pushback against this function with respect to safety:

  • The safe function is a total function, not a partial function
  • The safe function only makes its argument less partial. It's an improvement with respect to safety
  • Even for the counter-examples given (e.g. safe (tail . tail)) the result is still safer than the original version without safe (e.g. tail . tail)

There is no scenario where the safe function makes the program less safe

@Profpatsch
Copy link

I find myself sighing a lot when I realize that there is no safeHead or headMay in base, and that I have to either add it to my prelude (and a corresponding hlint rule for head), add the safe library, or match manually (not as easy for tail, though that one is questionable from a performance standpoint anyway).

@andreasabel
Copy link
Member

@Profpatch wrote:

I find myself sighing a lot when I realize that there is no safeHead or headMay in base,

It is called listToMaybe (maybe not the best name, but its the inverse of maybeToList).
You can discover it via its type: https://hoogle.haskell.org/?hoogle=%5Ba%5D%20-%3E%20Maybe%20a

@andreasabel
Copy link
Member

I downvoted the proposal: safe is too generic a name for the specific task.
FWIW, lots of safe projections from lists are defined in https://hackage.haskell.org/package/Agda-2.6.2.2/docs/Agda-Utils-List.html.
I also like a Null class with ifNull :: Null a => a -> b -> (a -> b) -> b (see https://hackage.haskell.org/package/Agda-2.6.2.2/docs/Agda-Utils-Null.html#v:ifNull) so that safe f xs = ifNull xs Nothing (Just . f).

@Gabriella439
Copy link
Author

@andreasabel: Would you approve a different name?

@treeowl
Copy link

treeowl commented May 29, 2022

Here's one approach:

{-# language MultiParamTypeClasses #-}
{-# language DataKinds #-}
{-# language TypeFamilies #-}
{-# language TypeApplications #-}
{-# language AllowAmbiguousTypes #-}
{-# language UndecidableInstances #-}

module SillySafe where
import GHC.TypeLits (Symbol)

class May (n :: Symbol) f where
  may :: f

instance f ~ ([a] -> Maybe a) => May "head" f where
  may [] = Nothing
  may (a : _) = Just a

instance f ~ ([a] -> Maybe [a]) => May "tail" f where
  may [] = Nothing
  may (_ : as) = Just as

instance (f ~ ([a] -> Maybe a), Ord a) => May "maximum" f where
  may as = foldr go id as Nothing
    where
      go x r Nothing = r (Just x)
      go x r old@(Just biggest)
        | x <= biggest = r old
        | otherwise = r (Just x)

-- Examples

may @"head" :: [a] -> Maybe a
may @"tail" :: [a] -> Maybe [a]
may @"maximum" :: Ord a => [a] -> Maybe a

I'd much rather use Language.Haskell.TH.Name than Symbol, to properly and ergonomically explain which tail I mean, but GHC inexplicably gives an error if I put something like 'tail in a type.

@tomjaguarpaw
Copy link
Member

I genuinely do not understand the pushback against this function with respect to safety

I don't believe that it will actually make an observable nudge towards making people write less crashing (more "safe") code in Haskell. I've suggested some alternatives that I think would:

  1. Adding the non-crashing versions to Prelude
  2. Adding stark warnings to the Haddocks of crashing functions
  3. Removing crashing functions from Prelude

I hope that helps explain my point of view.

@Gabriella439
Copy link
Author

Gabriella439 commented May 29, 2022

I would like to provide some critical feedback here because one aspect of this process is frustrating for me.

Could people please stop proposing wildly different solutions on this thread (such as #69 (comment))? Sorry to single you out @treeowl but you're not the only one.

I'm okay with suggesting and discussing related alternatives, including, but not limited to:

  • adding safeHead and friends directly instead of safe
  • suggesting a different name
  • (hypothetically) perhaps a minor variation on how the safe function is implemented

However, when people propose radically different solutions on this thread it derails this discussion in a few ways:

  • I cannot properly rebut those counter-proposals because they were never formally proposed, meaning that:
    • Nobody has volunteered to implement those proposals and see them through to their conclusion
    • Nobody has drafted a full issue that permits people to collect and review the potential tradeoffs of their counter-proposal
    • Every counter-proposal sounds good in theory until it is properly vetted and put to a vote by the core library committee
  • This thread cannot properly review those counter-proposals in their own right if they significantly differ from this proposal
    • This thread is already noisy enough as it is just reviewing the tradeoffs of this proposal and related proposals

More generally, all of these drive-by counter-proposals are unfair to those of us who actually do the work to submit formal proposals and volunteer to implement them. If you have a significantly different counter-proposal that you think would be a better solution, then open an issue to formally propose it.

@treeowl
Copy link

treeowl commented May 29, 2022

Fine. I don't like this proposal because:

  1. Its name doesn't adequately explain its purpose. Is there a way to make a concise name that does so? Maybe, but I don't have one to suggest.
  2. I'm not convinced it's sufficiently general to add to Data.List. As has been mentioned, it only makes very particular sorts of functions safe. Thus I'd prefer the safeSoAndSo or soAndSoMay approaches.

@thielema
Copy link

thielema commented May 29, 2022 via email

@tomjaguarpaw
Copy link
Member

Maybe we should not discuss proposals, but goals - at least first.

I'm inclined to agree. It's very hard to judge a proposal out of a broader context which includes a clear problem statement, a description of the status quo, and an analysis of the pros and cons of a wide range of solutions.

@NorfairKing
Copy link

NorfairKing commented May 29, 2022

The safe function would probably go on the dangerous function list under "doesn't do what you think it does", because it doesn't make a function safe (nor total, which is what you meant, I think).
See the safe (tail . tail) example.
There's also the issue that this wouldn't do what you want at all:

let listToTuple [x, y] = (x, y)
in safe listToTuple

I'd be happy with something with a better name, but at that point it's going to be more difficult to explain why you have to use onNotEmpty head, i.e. whyhead is not total already.
I'd prefer to get rid of the partial functions in base instead of bandaid-ing them up.

@Gabriella439
Copy link
Author

I'm okay with the proposal to add safeHead and friends directly, but I don't think the proposal to get rid of the partial functions in base is realistic, given that nobody has volunteered to actually do it or even to just lead the effort (and take care of all the migration/communication/documentation involved)

@Gabriella439
Copy link
Author

Also, going back to my comment here: #69 (comment)

… please do not propose removing partial functions from base in this thread unless someone has formally submitted a proposal to do so. The reason why is that it's not even clear what that even entails because it is a vague proposal without additional details and there are many ways to interpret that. For example:

  • What would be the migration strategy be?
  • Would the new total functions have the same name as the old partial functions or different names?
  • How long would the deprecation cycle be?

I cannot properly rebut that proposal unless people spell out what they mean by a separate issue to propose the change to remove partial functions from base

@treeowl
Copy link

treeowl commented May 29, 2022

As an aside, removing the partial functions might annoy users of Liquid Haskell, who use them quite safely.

@Bodigrim
Copy link
Collaborator

A point of order. Discussing alternative solutions is an important step, especially if a proposal does not cover them. However, a proposer is not obliged to analyze them in full or write detailed rebuttal indeed. A proposer can ask CLC to vote as is.

@Fresheyeball
Copy link

Call it nothingIfNull and I would support it. safe just doesn't capture what this function does, and much of the debate here is about if safe is appropriate. It doesn't make things safe generally, but it does supply a lambda where that assumes the list is non-empty, that's a cool thing. I would love to see this in Data.Foldable.

@tonyday567
Copy link

The proposal has a lot going for it.

The naming and ergonomics are a real sweet spot. Let's recall that right now, we have to tell new users that the current name for safe head is listToMaybe.

The alternative namings so far are nothingIfNull, onNotEmpty, safeSoAndSo, soAndSoMay, onNotNil, onNotNull, onNotEmpty, may & NonEmpty.head <$> nonEmpty. Please no.

Yes, usage of safe as a prefix has conventions around SafeHaskell, but that's a pretty minor convention compared with fixing usage of head. I can't imagine new users running into difficulty misunderstanding intent. If we really need to aim for purity of meaning, total would be an alternative spelling.

The safe (tail . tail) counter-example is not really convincing. safe tail . safe tail is safe and easy to type and explain.

The argument that we wouldn't do this because it's better to remove partial functions doesn't feel convincing. Even if true in theory, removal will never happen given current community resistance to change. In fact, if you dream of a total prelude, the proposal seems like a blessed pathway. Imagine:

  • safe is introduced to Prelude and this has no effect on existing code.
  • hlint starts to suggest adding safe to the site of every relevant Prelude partial function. Still no effect.
  • time goes by and safeHead gets added to Prelude. hlint starts to suggest changing safe tail with safeTail. Still no effect.

More time goes by, and we arrive at better common practice without harming our archeological heritage.

@tbidne
Copy link

tbidne commented May 30, 2022

The safe (tail . tail) counter-example is not really convincing. safe tail . safe tail is safe and easy to type and explain.

Well, it's not quite that nice. You'd need safe tail >=> safe tail.

@tonyday567
Copy link

tonyday567 commented May 30, 2022

How about a modification of the proposal to:

total :: Foldable f => (f a -> b) -> f a -> Maybe b
total f xs
    | null xs   = Nothing
    | otherwise = Just (f xs)

partial :: Foldable f => (f a -> b) -> f a -> b
partial  = id 

partial could be useful to signal intent to be unsafe, with a hlint default suggestion to replace head with partial head.

@Gabriella439
Copy link
Author

Alright, so it seems like the two leading counter-proposals are:

  • (A) Export safe versions of all the partial functions from the Prelude with some consistent naming convention (e.g. safeHead or headMay)

  • (B) Use another name that is more accurate

If it's alright with the core libraries committee, I can open up a new issue for (A) and if that issue is approved then I can close this one. I'm asking because I don't know if that's easier or more difficult for them to review it as an independent issue. Otherwise we can keep that discussion here.

For (B), the name I would suggest to use instead of safe is something along the lines of nullSafe or nullTotal, because I believe it addresses the criticisms of the name safe. Specifically, it accurately denotes that the function only provides additional protection in the case where the input is null.

For example, then it's clear from the name that nullSafe (tail . tail) is only safe for an empty input list and doesn't provide protection for a 1-element input list (which is the input that crashes tail . tail). I believe this addresses the objection from @NorfairKing that the original safe name was potentially misleading.

@mixphix
Copy link
Collaborator

mixphix commented May 31, 2022

Pinging @mstksg of nonempty-containers. This library has the approach I prefer most: specifically nonEmpty (which exists in base for NonEmpty) and withNonEmpty. Perhaps a HasNonEmpty typeclass would be better suited in base than a unitasker case for []?

@Gabriella439
Copy link
Author

@mixphix: Can you clarify what you are proposing? Specifically, how would you implement the safe head function under your proposal?

@hsenag
Copy link
Member

hsenag commented May 31, 2022

Could you provide the proposed haddock for your new function, along with fixing on a name at least for now? ITBH 'm instinctively against this proposal even regardless of whether any alternative is implemented, because I feel the new function will still be confusing to use and not really that useful. However the haddock could convince me otherwise.

@Gabriella439
Copy link
Author

I'll tentatively propose nullSafe as the new working name if nobody objects. My reasoning is that it reads more like natural English (people would be more likely to use the term "null-safe" to describe a function in ordinary conversation as opposed to, say, "null-total") and also the "safe" qualifier is less ambiguous when prefixed with "null" because now the type of safety is qualified (it is null safety and not unsafePerformIO-safety or some other form of safety).

Also, here are the following sample haddocks like @hsenag requests:

{-| Use `nullSafe` to wrap a partial function on lists (or, more generally, `Foldable`
    collections) if that function fails on an empty (`null`) input.  Examples of partial
    functions that benefit from being wrapped in this way are:

    * `head`
    * `tail`
    * `foldr1`
    * `maximum`

    For example:

    >>> head [2,3,5]
    2
    >>> head []
    *** Exception: Prelude.head: empty list
    >>> nullSafe head [2,3,5]
    Just 2
    >>> nullSafe head []
    Nothing

    This function does not fix all partial functions.  In particular,
    this does not fix functions that fail on non-`null` inputs.  For
    example, the `tail . tail` function fails on both an empty input
    and a 1-element input:

    >>> (tail . tail) [2,3,5]
    [5]
    >>> (tail. tail) [2]
    *** Exception: Prelude.tail: empty list
    >>> (tail . tail) []
    *** Exception: Prelude.tail: empty list

    … but if you were to wrap that in `nullSafe` then that would
    only fix the function for an empty input:

    >>> nullSafe (tail . tail) [2,3,5]
    Just [5]
    Prelude> nullSafe (tail . tail) [2]
    Just *** Exception: Prelude.tail: empty list
    Prelude> nullSafe (tail . tail) []
    Nothing

    The mnemonic is that a @`nullSafe` f@ is a \"null-safe f\" (e.g.
    \"null-safe head\" or \"null-safe tail\").
-}
nullSafe :: Foldable f => (f a -> b) -> f a -> Maybe b
nullSafe f xs
    | null xs   = Nothing
    | otherwise = Just (f xs)

@hsenag
Copy link
Member

hsenag commented Jun 2, 2022

Thanks - I think the name and the haddocks are quite clear. I've thought about my general objection a bit more and I think it's really that I don't see much point in safeHead etc either. You can convert to a Maybe but then you still have to do something about that at which point you probably should have just pattern-matched on the list anyway. But since there seems to be a general feeling that safeHead is needed, nullSafe head seems equally good or maybe slightly better.

@andreasabel
Copy link
Member

nullSafe :: Foldable f => (f a -> b) -> f a -> Maybe b
nullSafe f xs
    | null xs   = Nothing
    | otherwise = Just (f xs)

The CPSed version of this is

ifNull :: Foldable f => f a -> b -> (f a -> b) -> b
ifNull xs b f
  | null xs = b
  | otherwise = f xs

nullSafe f xs = ifNull xs Nothing (Just . f)
ifNull xs b f = fromMaybe b $ nullSafe f xs

My tendency would be to go with the CPSed ifNull.

But also, Foldable f isn't the only thing giving you a null element/test, and for lists you could also use the generalization towards Monoid:

nullSafe :: (Eq a, Monoid a) => (a -> b) -> a -> Maybe b
nullSafe f a
  | a == mempty = Nothing
  | otherwise = Just $ f a

Both Foldable and Monoid ask for too much. You simply want to check for the null element of the structure. This is why we have a Null a class in our utils: https://github.com/agda/agda/blob/ee8bf1d90086d46a567a4ad646e40c3d66fb9df3/src/full/Agda/Utils/Null.hs

I am still not convinced of this proposal.

@Gabriella439
Copy link
Author

@hsenag: You can pattern match on the list instead of using safeHead / safeTail, but that doesn't work as well for other partial functions like init or last

@thielema
Copy link

thielema commented Jun 2, 2022 via email

@Gabriella439
Copy link
Author

What about maximum, though?

@Gabriella439
Copy link
Author

If it's alright with the committee, I'd like to pause this until #70 is reviewed since if that were accepted I would prefer that over my own proposal

@emilypi
Copy link
Member

emilypi commented Jun 18, 2022

I'd rather just deprecate partiality in base as well

@Bodigrim
Copy link
Collaborator

@Gabriella439 how would you like to proceed with this, given that #70 is blocked on a GHC proposal and #87 has been approved?

@Gabriella439
Copy link
Author

I'm fine with closing this

@Bodigrim Bodigrim added the withdrawn Withdrawn by proposer label Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
withdrawn Withdrawn by proposer
Projects
None yet
Development

No branches or pull requests