Closed Bodigrim closed 3 years ago
+1
+1
+1
+1
+1
+1
Thanks everyone, approved unanimously.
I'm probably way too late to do anything about this now, but I find it very surprising that this change was approved so easily. I have argued against adding HasCallStack
to standard library functions in the past (which is why it wasn't done before, I successfully beat this back at least once). The core of the objection is that call stacks are a debugging feature that should not be visible in the source code, let alone the types. Yes this feature is useful sometimes - but not useful enough to make it so visible in the types of standard library functions. In the Prelude no less! I'd much rather see us invest in making the other methods for getting call stacks more convenient and accurate, reducing the need for HasCallStack
.
@goldfirere proposed IsPartial
as a denotational (as opposed to operational) alternative to HasCallStack
. That would allow partial functions to be marked without having to leak the specific implementation detail of the marking. partialityIsOK
(tweaked by me)
can intermediate between the two.
partialityIsOK :: HasCallStack => String -> (IsPartial => r) -> r
@tomjaguarpaw I would highly advise that we ask the PureScript community about their usage and experience of their own Partial
denotational typeclass. @hdgarrood Is this something on which you can comment?
@simonmar During the (very) heated debates, an alternative approach was suggested which was that we would have a Partial
constraint that (in GHC) would amount to a HasCallStack
constraint. This was suggested to enable stack traces in a way that doesn't force alternative implementation to use GHC's callstack.
IsPartial
is a bit better, but has the big disadvantage that it can't be enforced and would therefore end up being used inconsistently. The existence of IsPartial
might lead people to assume that the absence of IsPartial
implies total. Would we use IsPartial
for functions that might not terminate?
During the (very) heated debates,
I explored a bit but didn't find any very heated debates, can you help me with a link or two?
IsPartial is a bit better, but has the big disadvantage that it can't be enforced and would therefore end up being used inconsistently. The existence of IsPartial might lead people to assume that the absence of IsPartial implies total. Would we use IsPartial for functions that might not terminate?
Yes, those are hard questions that the PureScript community has to deal with, which is why I highly suggest that we survey them to see how their engineering practices have evolved on their side, to deal with Partial
.
We've definitely had people wondering whether the absence of a Partial constraint implies that a function is total, although off the top of my head I am not sure I remember whether I've seen people trip over this in their actual code. What I have seen, though, is that it's hard to explain how to use the class effectively. I wrote some documentation for the Partial class in PureScript but in the wild, in my experience, it's much more common to see people just slap our version of partialityIsOK
, namely unsafePartial
, on at the call site of each partial function, even if the call site has no basis for doing that and really the Partial constraint should have been propagated. If most people are misunderstanding how it's meant to be used and instead using it in this way, I think its utility over just documenting functions as partial is arguably limited.
Another thing that I think is worth mentioning, although which is possibly less applicable to Haskell than it is to PureScript, is that it's worth being clear about how the String
argument to partialityIsOK
is used, and how a partial function should behave in case it receives an argument for which it can't return a value. In PureScript, Partial
functions are not required to throw; for example, there is a partial array indexing function which returns undefined
at runtime if you ask for an out-of-bounds index (for performance reasons), even though most types do not permit undefined
as a valid runtime representation. For a while we had an unsafePartialBecause
function whose signature was forall a. String -> (Partial => a) -> a
, but it was deprecated it in favour of a version without the String
argument, because its implementation would catch and rethrow with the provided String
as a message, which encoded an invalid assumption that partiality was always indicated by throwing an exception. Of course, if that assumption were going to be valid in Haskell then it would be fine, and it would also be fine if the String
was ignored and was only present for the benefit of future readers of the code, although I personally find that slightly off-putting.
it's much more common to see people just slap our version of partialityIsOK, namely unsafePartial, on at the call site of each partial function, even if the call site has no basis for doing that and really the Partial constraint should have been propagated. If most people are misunderstanding how it's meant to be used and instead using it in this way, I think its utility over just documenting functions as partial is arguably limited.
I think my proposed partialityIsOK
deals with that, by requiring HasCallStack
(but I may be wrong because I don't really understand HasCallStack
).
partialityIsOK :: HasCallStack => String -> (IsPartial => r) -> r
I'm probably way too late to do anything about this now, but I find it very surprising that this change was approved so easily.
The proposal was raised in May and approved in October. This is as far from "easily" as I can imagine.
I explored a bit but didn't find any very heated debates, can you help me with a link or two?
Please refer to the links in https://github.com/haskell/core-libraries-committee/issues/5#issue-1037899194.
I would like to highlight that there was plenty of time to discuss and propose alternatives (IsPartial
, etc.). CLC has approved a specific design !6772.
I'm not complaining that I wasn't consulted, I mean sure, if I don't actively read the libraries mailing list then I'm sure I'll miss things. No, my surprise was really that nobody seemed to be worried about the aesthetics of the feature, and the impact on new users and teaching. As far as I can tell, my previous objection to this wasn't mentioned either - I just dug it up, I think this is it: https://gitlab.haskell.org/ghc/ghc/-/issues/11035#note_109305
@simonmar I guess the main difference is that when you raised your objections six years ago HasCallStack
was a relatively new concept, but now it is used by hundreds of packages (7512 matches across 412 packages), including base
itself. It's no longer experimental, it is widely adopted and well known. Somehow people prefer it to profiling builds and DWARF, and a better tooling for call stack has not materialized.
I think my proposed partialityIsOK deals with that, by requiring HasCallStack
Not quite. It certainly helps to have a call stack to hand if it does turn out to blow up at runtime, but what I had in mind was people misunderstanding how IsPartial
propagation is supposed to be used. For example, suppose you have a function foldr1 :: IsPartial => (a -> a -> a) -> [a] -> a
and you're trying to write a minimum
function using foldr1
. At first, you write this:
minimum :: [a] -> a
minimum = foldr1 min
This will fail because there's no IsPartial
instance available. At this stage, I've seen that it's common for people to not realise that they can add an IsPartial
constraint to minimum
, but instead just slap a partialityIsOK
on because they remember that that's how to shut the compiler up:
minimum :: [a] -> a
minimum = partialityIsOK "minimum should not be used with empty arrays" (foldr1 min)
despite the fact that the partiality has not been handled.
I think my proposed partialityIsOK deals with that, by requiring HasCallStack
Not quite.
Yes, I was thinking the IsPartial
constraint would just be swapped out for a HasCallStack
constraint, but HasCallStack
is magical and just gets satisfied if it doesn't appear in the annotated type.
Indeed, IsPartial
would be a breaking change, similar in effect to removal of head
from Prelude
.
I'm trying to summarise the state of this proposal as part of my volunteering effort to track the progress of all approved
CLC proposals.
Field | Value |
---|---|
Author | @AndreasPK |
Status | merged |
base version |
4.17.0.0 |
Merge Request (MR) | https://gitlab.haskell.org/ghc/ghc/-/merge_requests/6772 |
Blocked by | nothing |
CHANGELOG entry | present |
Migration guide | not needed |
Please, let me know if you find any mistakes 🙂
The original proposal was raised on May 30 by @phadej https://mail.haskell.org/pipermail/libraries/2021-May/031246.html, accompanied by detailed impact analysis. There were extensive discussions on the proposal in June. On Jul 7 @phadej asked CLC to trigger a vote on the proposal in https://mail.haskell.org/pipermail/libraries/2021-July/031355.html.
Merge request is available at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/6772. GHC developers seem to be on board with it.
Since CLC vote is long overdue, I would like to trigger it immediately. Members of CLC are encouraged to vote in replies to this issue.