Discussion:
Deprecating fromIntegral
Niklas Hambüchen
2017-09-21 12:24:14 UTC
Permalink
This is not a joke.

I just found a bug in base (https://ghc.haskell.org/trac/ghc/ticket/14262):

import System.IO
hWaitForInput stdin 4294968296

This code should wait for 49.something days, but it waits for 1 second.

It is caused by a quick use of `fromIntegral` to "convert" `Int -> CInt`
(where CInt = Int32), causing an overflow.

I argue that `fromIntegral` causes unnoticed data corruption without any
warning, and thus are worse than partial functions.

In this case, this data corruption occurs in the most fundamental code
that we all use and have to rely upon (`base`).

So I propose that we add a deprecation pragma to `fromIntegral`
(however, keeping it forever, like Java does, as there's no benefit to
removing it, we just want that people use safer functions over time),
and instead provide a `safeFromIntegral` that can only widen ranges, and
one or more `unsafeFromIntegral` or appropriately named functions that
can error, wrap, return Maybe, or have whatever desired behaviour when
the input value doesn't fit into the output type.

For some inspiration, `foundation` provides ideas in that direction:

http://hackage.haskell.org/package/basement-0.0.2/docs/Basement-From.html
http://hackage.haskell.org/package/basement-0.0.2/docs/Basement-IntegralConv.html

It is a bit funny how we argue that Haskell is good for high assurance
programming, and we build all sorts of fancy constructs to eliminate
more run-time errors, when in fact we do worse than C in detecting
errors in the most basic types a computer can use, such as converting
between 64-bit and 32-bit integers.

Let's fix that.

It avoids real problems in real production systems that are difficult to
debug when they happen (who writes a unit test checking that a timeout
of 50 days still works? This is the kind of stuff where you have to rely
on -- very basic -- types).

I'd like to probe support for such a change before doing any work on it.

Niklas
a***@gmail.com
2017-09-21 12:35:08 UTC
Permalink
I'm -1 on this (there are lots of places where you need to know the bounds of the numeric type you're using), but would be fine with adding more documentation to hWaitForInput and/or fromIntegral.

Tom
Post by Niklas Hambüchen
This is not a joke.
import System.IO
hWaitForInput stdin 4294968296
This code should wait for 49.something days, but it waits for 1 second.
It is caused by a quick use of `fromIntegral` to "convert" `Int -> CInt`
(where CInt = Int32), causing an overflow.
I argue that `fromIntegral` causes unnoticed data corruption without any
warning, and thus are worse than partial functions.
In this case, this data corruption occurs in the most fundamental code
that we all use and have to rely upon (`base`).
So I propose that we add a deprecation pragma to `fromIntegral`
(however, keeping it forever, like Java does, as there's no benefit to
removing it, we just want that people use safer functions over time),
and instead provide a `safeFromIntegral` that can only widen ranges, and
one or more `unsafeFromIntegral` or appropriately named functions that
can error, wrap, return Maybe, or have whatever desired behaviour when
the input value doesn't fit into the output type.
http://hackage.haskell.org/package/basement-0.0.2/docs/Basement-From.html
http://hackage.haskell.org/package/basement-0.0.2/docs/Basement-IntegralConv.html
It is a bit funny how we argue that Haskell is good for high assurance
programming, and we build all sorts of fancy constructs to eliminate
more run-time errors, when in fact we do worse than C in detecting
errors in the most basic types a computer can use, such as converting
between 64-bit and 32-bit integers.
Let's fix that.
It avoids real problems in real production systems that are difficult to
debug when they happen (who writes a unit test checking that a timeout
of 50 days still works? This is the kind of stuff where you have to rely
on -- very basic -- types).
I'd like to probe support for such a change before doing any work on it.
Niklas
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Niklas Hambüchen
2017-09-21 12:53:22 UTC
Permalink
Post by a***@gmail.com
I'm -1 on this (there are lots of places where you need to know the bounds of the numeric type you're using)
I don't quite understand what you mean with "you need to know the
bounds", or what role `fromIntegral` has in that, can you elaborate?
Niklas Hambüchen
2017-09-21 13:10:44 UTC
Permalink
Also, just to make that clear, the purpose here is not to to remove a
function that does something useful without providing a replacement.

It is to make sure that functions have appropriate names for what they
do, so that you don't use a function accidentally that has different
behaviour from what you expect.

For example, in the use of `ready` in GHC's Handle code


https://github.com/ghc/ghc/blob/ghc-8.2.1-release/libraries/base/GHC/IO/FD.hs#L399

the author certainly did not intend to have any wrapping behaviour, but
nothing in that code looks wrong unless you think really, really hard.

Because it looks correct.

(And has looked correct for 17 years:
https://github.com/ghc/ghc/blame/b83cfb91b4d36d148ebe171d31e2676c8f10f371/libraries/base/GHC/IO.hsc#L61)

If you wanted wrapping behaviour, a `wrapIntegral` would be your friend.

If a `wrapIntegral` is used in a place that shouldn't have it, that gets
noticed immediately in review.
Post by Niklas Hambüchen
Post by a***@gmail.com
I'm -1 on this (there are lots of places where you need to know the bounds of the numeric type you're using)
I don't quite understand what you mean with "you need to know the
bounds", or what role `fromIntegral` has in that, can you elaborate?
Henning Thielemann
2017-09-21 13:21:37 UTC
Permalink
Post by Niklas Hambüchen
It is a bit funny how we argue that Haskell is good for high assurance
programming, and we build all sorts of fancy constructs to eliminate
more run-time errors, when in fact we do worse than C in detecting
errors in the most basic types a computer can use, such as converting
between 64-bit and 32-bit integers.
That's all very true! Other safety oriented languages like Modula-3 at
least perform a runtime check on integer conversion.

However, I'd like to see practical solutions first before adding
deprecations.

fromInteger will always be partial when converting to a fixed width
integer. fromInteger is used for converting number literals to the wanted
type. Thus every number literal will issue a warning and I cannot see how
to circumvent this.

Another problem I see is that integralUpsize and integralDownsize are not
Haskell 98 and multi-parameter type classes are not really satisfying here
because you have to define n^2 instances for n types. A convenient
conversion function would certainly need even more advanced types, e.g.
type-level comparison of bit widths.

If we have fixed fromIntegral then the next natural question would arise:
How to protect arithmetic operations plus, minus, times against overflow?
Aren't overflows as dangerous as losing bits on integer conversion? Is
there a solution that works both for conversion and arithmetics?

For now the least invasive solution would be to provide a partial integer
conversion function.
Post by Niklas Hambüchen
So I propose that we add a deprecation pragma to `fromIntegral`
(however, keeping it forever, like Java does, as there's no benefit to
removing it, we just want that people use safer functions over time),
and instead provide a `safeFromIntegral` that can only widen ranges, and
one or more `unsafeFromIntegral` or appropriately named functions that
can error, wrap, return Maybe, or have whatever desired behaviour when
the input value doesn't fit into the output type.
As always, I object to use the terms 'safe' and 'unsafe' when actually
'total' and 'partial' are meant. LLVM uses 'ext' and 'trunc'. I'd prefer
names along these lines.
Niklas Hambüchen
2017-09-21 15:34:07 UTC
Permalink
Hey Henning,
Post by Henning Thielemann
That's all very true! Other safety oriented languages like Modula-3 at
least perform a runtime check on integer conversion.
However, I'd like to see practical solutions first before adding
deprecations.
I agree, deprecating before finding a solution doesn't make sense.

I intend this thread to be mainly about discussing possible solutions
and problems with them, hopefully we'll find something good.
Post by Henning Thielemann
fromInteger will always be partial when converting to a fixed width
integer. fromInteger is used for converting number literals to the
wanted type. Thus every number literal will issue a warning and I cannot
see how to circumvent this.
Yes. I would say though that `fromInteger` being partial is a less
severe problem than `fromIntegral`'s silent overflow; if a `fromInteger`
fails partially, you'd at least get a nice run-time error, which would
be better.

(Though, in practice the current `fromInteger is not partial, e.g.
`(fromInteger 256 :: Word8) == 0` without error, and I suspect it would
be good if we added a partial function in addition to the silently
wrapping one here too.)
Post by Henning Thielemann
Another problem I see is that integralUpsize and integralDownsize are
not Haskell 98 and multi-parameter type classes are not really
satisfying here because you have to define n^2 instances for n types. A
convenient conversion function would certainly need even more advanced
types, e.g. type-level comparison of bit widths.
That's a good point, and that's why I think that...
Post by Henning Thielemann
For now the least invasive solution would be to provide a partial
integer conversion function.
... this might be a good first step.

It would turn

"You ship code into production and 50 days later it corrupts your data"

into

"You ship code into production and 50 days later it crashes"

which is already much better, and, I believe, easy to implement.
Post by Henning Thielemann
If we have fixed fromIntegral then the next natural question would
arise: How to protect arithmetic operations plus, minus, times against
overflow? Aren't overflows as dangerous as losing bits on integer
conversion? Is there a solution that works both for conversion and
arithmetics?
Yes, all types of overflows can be problematic and dangerous.

You could say though that overflows "within the type" are somewhat less
problematic: Arithmetic overflows are more well-known and in people's
minds, it is relatively easy for a programmer to pick a type that is
large enough so that "+" doesn't overflow, and that only stops working
once you need to convert to another type, when the `fromIntegral`
problem occurs.
In "their own types", people work confidently, it's when interfacing
with others' code where the big bugs occur (like in the example I gave).

Haskell provides easy methods to prevent overflow *within* one's own
numeric types if one desires (like implementing "+" with `error` if it
doesn't fit); but currently our conversion functions *between* types
don't give us good explicit means to do so.

Also, how to deal with in-type overflows is an unsolved problem in
general, while C already solves much of the conversion problem.

I think solving the parts that C has solved would already avoid a good
amount of bugs.
Post by Henning Thielemann
As always, I object to use the terms 'safe' and 'unsafe' when actually
'total' and 'partial' are meant. LLVM uses 'ext' and 'trunc'. I'd prefer
names along these lines.
Good point.

I used "safe" and "unsafe" to express the idea that one kind would
always succeed in the naturally expected fashion and one would not, but
the more precisely the naming is, the better.
Henning Thielemann
2017-09-21 15:42:12 UTC
Permalink
Post by Niklas Hambüchen
It would turn
"You ship code into production and 50 days later it corrupts your data"
into
"You ship code into production and 50 days later it crashes"
which is already much better, and, I believe, easy to implement.
I thought that it would abort immediately, wouldn't it?
Niklas Hambüchen
2017-09-21 16:02:53 UTC
Permalink
Post by Henning Thielemann
I thought that it would abort immediately, wouldn't it?
Yes, I did a bit of a simplification here; certainly an overflow would
either create "corrupt"ed data in memory immediately or (better) abort
immediately.

I was thinking about the case where you deploy code and after it running
for a while (e.g. tracking elapsed milliseconds over 50 days of uptime)
something overflows and either corrupts or aborts.
Herbert Valerio Riedel
2017-09-22 12:14:12 UTC
Permalink
...there's one minor detail I'd like to point out as it doesn't seem
Post by Niklas Hambüchen
(Though, in practice the current `fromInteger is not partial, e.g.
`(fromInteger 256 :: Word8) == 0` without error
That's true. However, here's something you can do already now with
recent GHCs which gives turns literal-overflows GHC is able to detect into
hard compile errors:


$ ghci -Wall -Werror=overflowed-literals
GHCi, version 8.2.1: http://www.haskell.org/ghc/ :? for help
Prelude> (256 :: Data.Word.Word8) == 0

<interactive>:1:2: warning: [-Woverflowed-literals]
Literal 256 is out of the GHC.Word.Word8 range 0..255

<no location info>: error:
Failing due to -Werror.
Prelude>
Herbert Valerio Riedel
2017-09-21 14:22:38 UTC
Permalink
On 2017-09-21 at 14:24:14 +0200, Niklas Hambüchen wrote:

[...]
Post by Niklas Hambüchen
I argue that `fromIntegral` causes unnoticed data corruption without any
warning, and thus are worse than partial functions.
[...]
Post by Niklas Hambüchen
So I propose that we add a deprecation pragma to `fromIntegral`
Tbh, given how ubiquitous the use of `fromIntegral` is throughout the
Haskell ecosystem, having 'fromIntegral' suddely emit warnings when
`-Wall` is active is not realistic IMHO.

You'd have to introduce a separate (opt-in) category of WARNING pragmas
for that (something some of us would also like to see for the category
of partial functions) which isn't implied by -Wall.
Post by Niklas Hambüchen
(however, keeping it forever, like Java does, as there's no benefit to
removing it, we just want that people use safer functions over time),
and instead provide a `safeFromIntegral` that can only widen ranges, and
one or more `unsafeFromIntegral` or appropriately named functions that
can error, wrap, return Maybe, or have whatever desired behaviour when
the input value doesn't fit into the output type.
[...]
Post by Niklas Hambüchen
It avoids real problems in real production systems that are difficult to
debug when they happen (who writes a unit test checking that a timeout
of 50 days still works? This is the kind of stuff where you have to rely
on -- very basic -- types).
I ran into this very problem early on. I'll use the opportunity to
shamelessly plug an older package of mine probably only few know about
which solved this very problem in mission critical systems for me while
avoiding to have to enumerate O(n^2) transitions explicitly:

http://hackage.haskell.org/package/int-cast-0.1.2.0/docs/Data-IntCast.html

But it isn't plain Haskell 2010...

Another related package which may be useful in this context is
http://hackage.haskell.org/package/safeint which helps avoid silent
integer overflows during arithemtic operations.
Niklas Hambüchen
2017-09-21 15:57:06 UTC
Permalink
Hey Herbert,
Post by Herbert Valerio Riedel
I ran into this very problem early on. I'll use the opportunity to
shamelessly plug an older package of mine probably only few know about
which solved this very problem in mission critical systems for me while
http://hackage.haskell.org/package/int-cast-0.1.2.0/docs/Data-IntCast.html

This is very useful, thanks for that.
Post by Herbert Valerio Riedel
Tbh, given how ubiquitous the use of `fromIntegral` is throughout the
Haskell ecosystem, having 'fromIntegral' suddely emit warnings when
`-Wall` is active is not realistic IMHO.
Here I disagree.

It is "realistic" because we can just do it, if we want.

The questions more if we want to.

I'd say yes, because:

Deprecation-marking does not break any code, it is the softest way to
make a transition. The Java community has this figured out: Deprecations
are common, they don't break the build, they don't change behaviour,
downstream libraries adapt quickly to them, and the whole ecosystem can
advance to a more solid foundation gracefully.

The way you phrase it sounds as if you'd expect a large backlash from
your users if you emitted these extra warnings.

But I think the opposite can be expected:
Haskell users would love if -Wall pointed out *more* ways in which their
code may break.
We use -Wall because we *want* to be warned of slumbering bugs.

I think your users would cheer at you, not complain.

(Even the C people regularly cheer as new warnings make it into -Wall of
gcc and clang.)
Evan Laforge
2017-09-21 17:27:51 UTC
Permalink
Post by Niklas Hambüchen
Deprecation-marking does not break any code, it is the softest way to
make a transition. The Java community has this figured out: Deprecations
are common, they don't break the build, they don't change behaviour,
downstream libraries adapt quickly to them, and the whole ecosystem can
advance to a more solid foundation gracefully.
As I remember it, where I worked there was a distinction between
deprecations and warnings. Deprecations had no visible output outside
the documentation, unless you used an IDE and turned on the
dotted-underline feature, while warnings would be printed during the
compile. You were not supposed to deprecate anything without a
clearly documented "do this instead" but you didn't need a plan to fix
all the callers. But to emit a warning, a bounded-time plan was
required, which probably involved a global fix via automatic
refactoring.

Otherwise, warnings accumulate, and once they get past a certain
amount everyone ignores all of them equally, which is a loss as we
lose the high value ones too. Most of them come out of compiling
stuff you don't own and don't understand and don't have time to go
fix.

If I were to suddenly get 10,000 lines of warnings... well, if it can
be fixed with a global search and replace then it's ok if they only
indicate a few real problems. But if the fix is more complicated and
requires individually looking at each place, then they had better have
a pretty good positive rate. Otherwise I wind up turning off the
warning and it might as well not exist.

You could say there's a place for low value lint type warnings which
you could turn on just for hlint style suggestions, but it needs to be
integrated with the source control so it can warn only about the bits
that you just changed. Otherwise no one can repeatedly comb through
the 10,000 suggestions in case some have been added, so in practice it
just stays off forever.

Also in my experience the bit about "downstream libraries adapt
quickly to them" is wildly optimistic :)
Niklas Hambüchen
2017-09-21 23:19:49 UTC
Permalink
Post by Evan Laforge
If I were to suddenly get 10,000 lines of warnings...
If the built tool is set up sensibly (perhaps we should make it so if
that is not already possible, but I think these days warnings of
dependency packages are omitted anyway, as cabal and stack build them in
parallel), you should get (and care about and fix) only warnings about
those occurrences that are in the packages you maintain / build directly.

You are unlikely to get 10,000 warnings:

I've made a quick histogram over all of Hackage (note, this includes
lots of abanoned code)


https://gist.github.com/nh2/f45d628a81c04f95e14f14ea37d33b22#file-fromintegral-uses-png

There is also a JSON file containing counts for each packet so that you
can quickly look up how many occurrences your packages have.

Most packages have < 3 occurrences of fromIntegral, a few have < 20
occurrences. Very few have more than 20.

Assuming we go for a naming improvement and add 3 functions ala
maybeFromInteger / runtimeCheckedFromInteger / fromIntegerWrap, a
package owner would have to step through their deprecation warnings and
replace each with one of the new 3 functions to get their package
warning-free.

I estimate this to be a very minor task/effort compared to the major
changes GHC Haskell has gone through in the last years.

I could probably already have fixed 1000 of the 7000 total occurrences
on Hackage myself within the time it as taken me to debug integer
conversion related problems in GHC/base in the last week.

(In other words, these changes are cheap and have high payoff.)
Carter Schonwald
2017-09-22 11:30:42 UTC
Permalink
I’m -10 in this proposal

Every time I write a number literal I’ll now get a warning. Cause as it
hasn’t been remarked : every integer literal uses these operations afaik

The intent / spirit of this proposal is correct, the method of recourse
isn’t.
Post by Niklas Hambüchen
Post by Evan Laforge
If I were to suddenly get 10,000 lines of warnings...
If the built tool is set up sensibly (perhaps we should make it so if
that is not already possible, but I think these days warnings of
dependency packages are omitted anyway, as cabal and stack build them in
parallel), you should get (and care about and fix) only warnings about
those occurrences that are in the packages you maintain / build directly.
I've made a quick histogram over all of Hackage (note, this includes
lots of abanoned code)
https://gist.github.com/nh2/f45d628a81c04f95e14f14ea37d33b22#file-fromintegral-uses-png
There is also a JSON file containing counts for each packet so that you
can quickly look up how many occurrences your packages have.
Most packages have < 3 occurrences of fromIntegral, a few have < 20
occurrences. Very few have more than 20.
Assuming we go for a naming improvement and add 3 functions ala
maybeFromInteger / runtimeCheckedFromInteger / fromIntegerWrap, a
package owner would have to step through their deprecation warnings and
replace each with one of the new 3 functions to get their package
warning-free.
I estimate this to be a very minor task/effort compared to the major
changes GHC Haskell has gone through in the last years.
I could probably already have fixed 1000 of the 7000 total occurrences
on Hackage myself within the time it as taken me to debug integer
conversion related problems in GHC/base in the last week.
(In other words, these changes are cheap and have high payoff.)
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Niklas Hambüchen
2017-09-22 12:58:25 UTC
Permalink
I’m -10 in this proposal
Every time I write a number literal I’ll now get a warning.
Hi Carter, is this true?

It was my understanding that if you deprecate a function, you get
deprecation warnings only where you make literal use of that function, e.g.

x = 10

would not raise a deprecation because you didn't explicitly write
"fromInteger" here (and certainly not fromIntegral).

Is this not the case?
Cause as it hasn’t been remarked : every integer literal uses these
operations afaik

Both Henning and Edward have remarked that.
Evan Laforge
2017-09-22 18:53:58 UTC
Permalink
Post by Niklas Hambüchen
Post by Evan Laforge
If I were to suddenly get 10,000 lines of warnings...
If the built tool is set up sensibly (perhaps we should make it so if
that is not already possible, but I think these days warnings of
dependency packages are omitted anyway, as cabal and stack build them in
parallel), you should get (and care about and fix) only warnings about
those occurrences that are in the packages you maintain / build directly.
I mean dependencies within the project, not external packages. If
modify something in the middle of the dependency tree, I'll be usually
recompiling (say) a couple of hundred modules, for my personal
medium-ish project. I assume in a real company with a larger codebase
you could easily be recompiling thousands. Maybe just the mono-repo
ones though.

Also I'm assuming most code is not in packages, and is not in hackage.
I have no real basis for that, other than experience with other
Post by Niklas Hambüchen
Most packages have < 3 occurrences of fromIntegral, a few have < 20
occurrences. Very few have more than 20.
Assuming we go for a naming improvement and add 3 functions ala
maybeFromInteger / runtimeCheckedFromInteger / fromIntegerWrap, a
package owner would have to step through their deprecation warnings and
replace each with one of the new 3 functions to get their package
warning-free.
I'm surprised about that, I assumed more. Grepping for fromIntegral
in a local project, I get 259 occurrences over 693 modules... I
expected a lot more.

However, a lot of those I think would be false positives, because
they're converting across newtypes, where the underlying type is the
same. I'm not sure what I would use for those... I guess it would be
fromIntegerWrapped, but it's kind of verbose and awkward and is
implying wrapping when it's not actually present. Maybe they should
be using 'coerce'? Most of them existed before coerce did, but that's
not an excuse any more. I think I'd be more likely to try out
int-cast.
Post by Niklas Hambüchen
I could probably already have fixed 1000 of the 7000 total occurrences
on Hackage myself within the time it as taken me to debug integer
conversion related problems in GHC/base in the last week.
(In other words, these changes are cheap and have high payoff.)
Of course that's the bottom line, so if it's the case then it's hard
to argue against.

I've had similar bugs due to sloppy haskell -> C FFI conversions so
I'm sympathetic to the idea. It's just a blanket deprecation on
fromIntegral seems like a broad brush.
Henning Thielemann
2017-09-23 21:28:29 UTC
Permalink
Post by Evan Laforge
However, a lot of those I think would be false positives, because
they're converting across newtypes, where the underlying type is the
same. I'm not sure what I would use for those... I guess it would be
fromIntegerWrapped, but it's kind of verbose and awkward and is
implying wrapping when it's not actually present. Maybe they should
be using 'coerce'?
Why not just use the newtype constructor or field accessor for packing and
unpacking? Usually I wrap Ints in newtypes for identifiers. Identifiers
should not support Num arithmetic and thus would not allow fromIntegral
anyway.
Henning Thielemann
2017-09-23 21:31:11 UTC
Permalink
I've had similar bugs due to sloppy haskell -> C FFI conversions so I'm
sympathetic to the idea. It's just a blanket deprecation on
fromIntegral seems like a broad brush.
I assume that conversion causes problems depending on the architecture.
E.g. Int -> CInt might be a lossless conversion on a 32-bit architecture
and lossy on 64-bit.
John Wiegley
2017-09-21 17:47:29 UTC
Permalink
NH> Here I disagree.
NH> It is "realistic" because we can just do it, if we want.
NH> The questions more if we want to.
NH> I'd say yes, because:

Why is it 'fromIntegral' that should be promoted into a warning? What about
all the other partial functions in base that don't generate warnings either?
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
Niklas Hambüchen
2017-09-21 21:30:51 UTC
Permalink
Post by John Wiegley
Why is it 'fromIntegral' that should be promoted into a warning? What about
all the other partial functions in base that don't generate warnings either?
Because `fromIntegral` is not a partial function, it fails silently.

Also because fixing all partial functions in base is way out of scope of
what I intended to propose.
John Wiegley
2017-09-21 22:01:48 UTC
Permalink
NH> Because `fromIntegral` is not a partial function, it fails silently.

Ah, yes, I didn't mean "partial" per se, but more in the sense of "not having
a sane answer for all inputs".

I'm -1 to changing the behavior of fromIntegral now, but +1 on adding other
functions to make the behavior more explicit.
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
Niklas Hambüchen
2017-09-21 22:12:50 UTC
Permalink
Post by John Wiegley
I'm -1 to changing the behavior of fromIntegral now
I never suggested to change the behaviour of fromIntegral.

I even argued against that twice:

* once in my original email (saying we should deprecate the current
fromIntegral but keep it forever for backwards compatibility)
* once in my reply to Edward ("unchanged semantics, but emitting a
deprecation warning")
Post by John Wiegley
but +1 on adding other
functions to make the behavior more explicit.
Then you are +1 on my proposal, because that is exactly what I wrote.
John Wiegley
2017-09-21 22:24:15 UTC
Permalink
NH> Then you are +1 on my proposal, because that is exactly what I wrote.

OK, thanks for clarifying then, and apologies for the added confusion.
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
a***@gmail.com
2017-09-22 23:07:16 UTC
Permalink
Post by John Wiegley
NH> Because `fromIntegral` is not a partial function, it fails silently.
Ah, yes, I didn't mean "partial" per se, but more in the sense of "not having
a sane answer for all inputs".
I'm -1 to changing the behavior of fromIntegral now, but +1 on adding other
functions to make the behavior more explicit.
This is my view. I am specifically against adding a {-# DEPRECATED #-} pragma.

Tom
Post by John Wiegley
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Edward Kmett
2017-09-21 14:41:43 UTC
Permalink
I'm -1 on this proposal.

fromInteger and fromIntegral are a huge part of our ecosystem. They
comprise a large part of the difference between how we handle numeric
literals and how literally every other language on the planet handles
numeric literals. fromIntegral shows up all over the place in any code that
is even remotely mathematical to deal with converting between integer
indices and other numeric types for computation.

If we pretend Num is about working with rings fromInteger provides the
canonical ring homomorphism from Z into every ring, so that half of the
equation has to remain in place. toInteger is Integral reason for
existence, so you're just asking the existing users to replace fromIntegral
with 'fromInteger . toInteger' do dodge a warning that doesn't come with
any other clear explanation about how to fix.

Any properly replacement requires language extensions such as multi
parameter type classes that we've failed to demonstrate an ability to
standardize as well as n^2 instances and makes it difficult to convert
between integral types and numbers supplied by different packages.

Now, if you wanted to write a package that provided widening and shrinking
conversions and this gradually was picked up by a larger and larger cross
section of the community and more organically worked its way into base
that'd be one thing, but these nicer safe conversions haven't yet been
adopted by any significant fraction of the community at this point, so any
form of deprecation is very much premature.

On the other hand I'm definitely +1 on adding documentation to
hWaitForInput or on fromIntegral about how you should be careful when
switching between numeric types of different sizes.

-Edward
Post by Niklas Hambüchen
This is not a joke.
I just found a bug in base (https://ghc.haskell.org/trac/ghc/ticket/14262
import System.IO
hWaitForInput stdin 4294968296
This code should wait for 49.something days, but it waits for 1 second.
It is caused by a quick use of `fromIntegral` to "convert" `Int -> CInt`
(where CInt = Int32), causing an overflow.
I argue that `fromIntegral` causes unnoticed data corruption without any
warning, and thus are worse than partial functions.
In this case, this data corruption occurs in the most fundamental code
that we all use and have to rely upon (`base`).
So I propose that we add a deprecation pragma to `fromIntegral`
(however, keeping it forever, like Java does, as there's no benefit to
removing it, we just want that people use safer functions over time),
and instead provide a `safeFromIntegral` that can only widen ranges, and
one or more `unsafeFromIntegral` or appropriately named functions that
can error, wrap, return Maybe, or have whatever desired behaviour when
the input value doesn't fit into the output type.
http://hackage.haskell.org/package/basement-0.0.2/docs/Basement-From.html
http://hackage.haskell.org/package/basement-0.0.2/docs/
Basement-IntegralConv.html
It is a bit funny how we argue that Haskell is good for high assurance
programming, and we build all sorts of fancy constructs to eliminate
more run-time errors, when in fact we do worse than C in detecting
errors in the most basic types a computer can use, such as converting
between 64-bit and 32-bit integers.
Let's fix that.
It avoids real problems in real production systems that are difficult to
debug when they happen (who writes a unit test checking that a timeout
of 50 days still works? This is the kind of stuff where you have to rely
on -- very basic -- types).
I'd like to probe support for such a change before doing any work on it.
Niklas
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Niklas Hambüchen
2017-09-21 17:26:45 UTC
Permalink
Hey Edward,

I'm not going after Num here.

What I care about is that base provide conversion functions with clear
names that indicate what they do, so that the intent of the author
becomes obvious.

Currently we cram all number conversion functionality into
`fromIntegral`, and offer no standard way for talking about code that
has no intent to wrap.

This code:

fdReady ... (fromIntegral msecs)

does not look obviously incorrect, but this code would:

fdReady ... (fromIntegralWrap msecs)

Of course it would be great if a name change came along with a set of
type classes that make it compile-time-impossible to do an unintended
cast, but in absence of one agreed way to do that, more functions with
better names and appropriate error behaviour would already go a long way
for avoiding bugs like this.

To throw out some concrete examples, there could be:

* maybeFromInteger / maybeFromIntegral
- that return Nothing when the input doesn't fit
* runtimeCheckedFromInteger / runtimeCheckedFromIntegral
- `error` on Nothing
* fromIntegerWrap / fromIntegralWrap
- what the current functions do
* fromInteger / fromIntegral
- unchanged semantics, but emitting a deprecation warning

Deprecation seems the best method by which we can systematically
highlight all use sites where a replacement with a function of a better
name, or missing edge case handling, should be inserted, without
breaking code.

Niklas
Edward Kmett
2017-09-21 17:57:07 UTC
Permalink
Note:

hWaitForInput stdin 4294968296

is using fromInteger, not fromIntegral. fromInteger is the member of Num.
fromIntegral is defined in terms of it and toInteger.

Deprecating fromIntegral would do nothing to the example you offered at all
as it isn't being called.

Like it or not the existing numeric types in the Haskell ecosystem wrap,
and changing this behavior would have non-trivial ramifications for almost
all Haskell source code written to date.

This isn't a small change you are proposing.

If your goal is to address this micro-issue, it would be a much easier
target to aim for hWaitForInput taking an Integer rather than an Int,
removing the issue at the source. There are already packages providing
safer delays, e.g.

https://hackage.haskell.org/package/unbounded-delays

It has gradually become clear that using Int in delay and hWaitForInput and
delay the like was a mistake. After all if you're going to set up to block
anyways the overhead of allocating a single Integer constructor is trivial.

For comparison, I have zero objection to going after the type signature of
hWaitForInput!

Changing fromInteger on the other hand is a Prelude affecting change that
bubbles all the way down into Num. I'm very much against breaking every
single Num instance, and making every Haskell developer audit every
application of a very common function to fix one overflow in one naively
designed system call.

-Edward
Post by Niklas Hambüchen
Hey Edward,
I'm not going after Num here.
What I care about is that base provide conversion functions with clear
names that indicate what they do, so that the intent of the author
becomes obvious.
Currently we cram all number conversion functionality into
`fromIntegral`, and offer no standard way for talking about code that
has no intent to wrap.
fdReady ... (fromIntegral msecs)
fdReady ... (fromIntegralWrap msecs)
Of course it would be great if a name change came along with a set of
type classes that make it compile-time-impossible to do an unintended
cast, but in absence of one agreed way to do that, more functions with
better names and appropriate error behaviour would already go a long way
for avoiding bugs like this.
* maybeFromInteger / maybeFromIntegral
- that return Nothing when the input doesn't fit
* runtimeCheckedFromInteger / runtimeCheckedFromIntegral
- `error` on Nothing
* fromIntegerWrap / fromIntegralWrap
- what the current functions do
* fromInteger / fromIntegral
- unchanged semantics, but emitting a deprecation warning
Deprecation seems the best method by which we can systematically
highlight all use sites where a replacement with a function of a better
name, or missing edge case handling, should be inserted, without
breaking code.
Niklas
Niklas Hambüchen
2017-09-21 22:02:15 UTC
Permalink
Post by Niklas Hambüchen
hWaitForInput stdin 4294968296
is using fromInteger, not fromIntegral.fromInteger is the member of Num.
fromIntegral is defined in terms of it and toInteger.
Deprecating fromIntegral would do nothing to the example you offered at
all as it isn't being called.
Ah, here is the misunderstanding.

It is not the call to to hWaitForInput that is overflowing.

hWaitForInput takes an `Int`, and so passing 4294968296 to it is
perfectly fine (on 64-bit platforms).

As described in the linked bug
https://ghc.haskell.org/trac/ghc/ticket/14262, the overflow is in the
code that implements hWaitForInput,

ready :: FD -> Bool -> Int -> IO Bool
ready fd write msecs = do
r <- throwErrnoIfMinus1Retry "GHC.IO.FD.ready" $
fdReady (fdFD fd) (fromIntegral $ fromEnum $ write)
(fromIntegral msecs)

in the `fromIntegral msecs`.

I provided the `hWaitForInput 4294968296` bit only to give an easily
reproducible example of how `fromIntegral` results in a significant bug.

I realise now that the `4294968296` is why you brought up the topic of
integer literals (which confused me a bit).
Post by Niklas Hambüchen
Like it or not the existing numeric types in the Haskell ecosystem wrap,
and changing this behavior would have non-trivial ramifications for
almost all Haskell source code written to date.
I did not propose to change the behaviour of any existing function.

I said:

"
* fromInteger / fromIntegral
- unchanged semantics, but emitting a deprecation warning
"

Niklas
d***@steinitz.org
2017-09-22 12:45:00 UTC
Permalink
Coincidentally, I was playing with this a few days ago. I didn’t seem to have to do anything to get a warning.
GHCi, version 8.0.2: http://www.haskell.org/ghc/ :? for help
Prelude Data.Word> :m Data.Int
Prelude Data.Int> 0x80000000 :: Int32
<interactive>:27:1: warning: [-Woverflowed-literals]
Literal 2147483648 is out of the Int32 range -2147483648..2147483647
If you are trying to write a large negative literal, use NegativeLiterals
-2147483648
Prelude Data.Int> :set -XNegativeLiterals
Prelude Data.Int> 0x80000000 :: Int32
<interactive>:29:1: warning: [-Woverflowed-literals]
Literal 2147483648 is out of the Int32 range -2147483648..2147483647
-2147483648
I am not clear why it wanted me to use NegativeLiterals but still give the same warning. Does anyone understand the intent here?
Message: 1
Date: Fri, 22 Sep 2017 14:14:12 +0200
Subject: Re: Deprecating fromIntegral
Content-Type: text/plain; charset="UTF-8"
...there's one minor detail I'd like to point out as it doesn't seem
Post by Niklas Hambüchen
(Though, in practice the current `fromInteger is not partial, e.g.
`(fromInteger 256 :: Word8) == 0` without error
That's true. However, here's something you can do already now with
recent GHCs which gives turns literal-overflows GHC is able to detect into
$ ghci -Wall -Werror=overflowed-literals
GHCi, version 8.2.1: http://www.haskell.org/ghc/ :? for help
Prelude> (256 :: Data.Word.Word8) == 0
<interactive>:1:2: warning: [-Woverflowed-literals]
Literal 256 is out of the GHC.Word.Word8 range 0..255
Failing due to -Werror.
Prelude>
------------------------------
Subject: Digest Footer
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
------------------------------
End of Libraries Digest, Vol 169, Issue 30
******************************************
Dominic Steinitz
***@steinitz.org
http://idontgetoutmuch.wordpress.com
Thomas Jakway
2017-09-25 21:33:18 UTC
Permalink
I can see both sides of this and am dismayed. On the one hand, -Wall
literally means with all warnings--so it makes sense to pack as many
warnings as possible into it. If that's too many warnings for the end
user, they can disable individual ones when looking through the output.
If we wanted to be really thorough we could provide a suggestion at the
end of the output ("90% of your warnings are from ____"). On the other
hand,

grep -I -R --include="*.hs" "fromIntegral" | wc -l

on ghc prints 2598, which is a lot.

For what it's worth I'm in favor of adding it to -Wall because I would
rather be warned about too many things and have to force myself not to
ignore them than spend a long time figuring out the source of a bug. At
the very least when I do encounter a bug it'll give me somewhere to start.

There's no great solution here, only tradeoffs. I won't lose any faith
in Haskell whichever route we take.
Post by Evan Laforge
Post by Niklas Hambüchen
Post by Evan Laforge
If I were to suddenly get 10,000 lines of warnings...
If the built tool is set up sensibly (perhaps we should make it so if
that is not already possible, but I think these days warnings of
dependency packages are omitted anyway, as cabal and stack build them in
parallel), you should get (and care about and fix) only warnings about
those occurrences that are in the packages you maintain / build
directly.
Post by Evan Laforge
I mean dependencies within the project, not external packages. If
modify something in the middle of the dependency tree, I'll be usually
recompiling (say) a couple of hundred modules, for my personal
medium-ish project. I assume in a real company with a larger codebase
you could easily be recompiling thousands. Maybe just the mono-repo
ones though.
Also I'm assuming most code is not in packages, and is not in hackage.
I have no real basis for that, other than experience with other
Post by Niklas Hambüchen
Most packages have < 3 occurrences of fromIntegral, a few have < 20
occurrences. Very few have more than 20.
Assuming we go for a naming improvement and add 3 functions ala
maybeFromInteger / runtimeCheckedFromInteger / fromIntegerWrap, a
package owner would have to step through their deprecation warnings and
replace each with one of the new 3 functions to get their package
warning-free.
I'm surprised about that, I assumed more. Grepping for fromIntegral
in a local project, I get 259 occurrences over 693 modules... I
expected a lot more.
However, a lot of those I think would be false positives, because
they're converting across newtypes, where the underlying type is the
same. I'm not sure what I would use for those... I guess it would be
fromIntegerWrapped, but it's kind of verbose and awkward and is
implying wrapping when it's not actually present. Maybe they should
be using 'coerce'? Most of them existed before coerce did, but that's
not an excuse any more. I think I'd be more likely to try out
int-cast.
Post by Niklas Hambüchen
I could probably already have fixed 1000 of the 7000 total occurrences
on Hackage myself within the time it as taken me to debug integer
conversion related problems in GHC/base in the last week.
(In other words, these changes are cheap and have high payoff.)
Of course that's the bottom line, so if it's the case then it's hard
to argue against.
I've had similar bugs due to sloppy haskell -> C FFI conversions so
I'm sympathetic to the idea. It's just a blanket deprecation on
fromIntegral seems like a broad brush.
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
David Feuer
2017-09-25 21:35:39 UTC
Permalink
I'm -1 on deprecating it, and +1 on adding additional conversion functions
of various sorts: explicit widening, explicit narrowing, etc.
Post by Niklas Hambüchen
This is not a joke.
I just found a bug in base (https://ghc.haskell.org/trac/ghc/ticket/14262
import System.IO
hWaitForInput stdin 4294968296
This code should wait for 49.something days, but it waits for 1 second.
It is caused by a quick use of `fromIntegral` to "convert" `Int -> CInt`
(where CInt = Int32), causing an overflow.
I argue that `fromIntegral` causes unnoticed data corruption without any
warning, and thus are worse than partial functions.
In this case, this data corruption occurs in the most fundamental code
that we all use and have to rely upon (`base`).
So I propose that we add a deprecation pragma to `fromIntegral`
(however, keeping it forever, like Java does, as there's no benefit to
removing it, we just want that people use safer functions over time),
and instead provide a `safeFromIntegral` that can only widen ranges, and
one or more `unsafeFromIntegral` or appropriately named functions that
can error, wrap, return Maybe, or have whatever desired behaviour when
the input value doesn't fit into the output type.
http://hackage.haskell.org/package/basement-0.0.2/docs/Basement-From.html
http://hackage.haskell.org/package/basement-0.0.2/docs/
Basement-IntegralConv.html
It is a bit funny how we argue that Haskell is good for high assurance
programming, and we build all sorts of fancy constructs to eliminate
more run-time errors, when in fact we do worse than C in detecting
errors in the most basic types a computer can use, such as converting
between 64-bit and 32-bit integers.
Let's fix that.
It avoids real problems in real production systems that are difficult to
debug when they happen (who writes a unit test checking that a timeout
of 50 days still works? This is the kind of stuff where you have to rely
on -- very basic -- types).
I'd like to probe support for such a change before doing any work on it.
Niklas
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Niklas Hambüchen
2017-09-26 15:08:35 UTC
Permalink
Hey David,
Post by David Feuer
I'm -1 on deprecating it, and +1 on adding additional conversion
functions of various sorts: explicit widening, explicit narrowing, etc.
What is your reasoning against deprecating it?

That `fromIntegral` is a function that should be used in new code?
That emitting a warning for potentially wrong code is bad by itself?
Or that people have to change their code to be warning free?

To me, a deprecation constitutes an organised, non-disruptive migration
from the old function to the new explicit functions, and I'd really like
to understand what people's reservations against it are.
Edward Kmett
2017-09-26 16:41:40 UTC
Permalink
To me deprecation is a big deal, especially for something exported from
Prelude and which has been around since before Haskell 98, when we simply
do *not* have a clear migration path for users at this point.

The way I see it there are several gates between this proposal and any
actual deprecation of fromIntegral.

1. At present there is no story here for how users change their code to
address the deprecation. They'd just get to stare at a -Wall full of
warnings and have no migration path.

2. To start a deprecation process here I'd at the very least want to have a
replacement that a decent cross-section of folks have been able to agree is
an improvement. This could be packaged up independently, could be selected
from among other solutions that folks have already packaged up, etc. I
don't care where it comes from so long as you can get enough people agree
that it is the right solution.

3. Build consensus to move it into base. That way to address any eventual
deprecation there can be a road map that doesn't incur new dependencies for
users.

4. Then once it has been around in base long enough that users can migrate
to it in accordance to the "3 Release Policy"
<https://prime.haskell.org/wiki/Libraries/3-Release-Policy> then we could
build consensus to perform a deprecation of the existing method.

This is unfortunately a relatively slow and tedious process, but the
alternative is a Prelude-affecting change that breaks our release policies
and all of the books being made with no real notice.

The current usage pattern and policies we have around deprecations isn't to
use them as the first notice of "hey you might want to do something else",
but rather to catch folks on the tail end right before something gets
removed and long after what the user should do to in response has been
carefully documented and considered.

At least it seems to me that this is a large part of the resistance you are
encountering here.

-Edward
Post by Niklas Hambüchen
Hey David,
Post by David Feuer
I'm -1 on deprecating it, and +1 on adding additional conversion
functions of various sorts: explicit widening, explicit narrowing, etc.
What is your reasoning against deprecating it?
That `fromIntegral` is a function that should be used in new code?
That emitting a warning for potentially wrong code is bad by itself?
Or that people have to change their code to be warning free?
To me, a deprecation constitutes an organised, non-disruptive migration
from the old function to the new explicit functions, and I'd really like
to understand what people's reservations against it are.
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
David Feuer
2017-09-26 17:00:37 UTC
Permalink
I would double down on the "no one has a great replacement yet".
There are several (potential, imaginary) replacements that may be
better in different contexts, depending on the precise programmer
intent. And then there's the question of how far we want to push
(relatives of) a function that doesn't really seem like entirely the
right idea. Completely aside from correctness issues, fromIntegral
also has efficiency trouble: it is completely dependent on
hand-written rewrite rules to avoid serious inefficiencies. A version
based on MultiParamTypeClasses would inspire much more confidence--but
it would require instances out the wazoo and surely lead to many
orphan instances. I suspect there are ways we can get better results
with fewer problems by designating several different intermediate
types and allowing Integral instances to place themselves in relation
to those. But that is ad hoc, and it requires one or more language
extensions, and I don't know if anyone's really played with it enough.
We shouldn't even be thinking about deprecation in the short term; we
should be working toward a medium-term replacement and a
very-long-term *potential* deprecation.
Post by Edward Kmett
To me deprecation is a big deal, especially for something exported from
Prelude and which has been around since before Haskell 98, when we simply do
not have a clear migration path for users at this point.
The way I see it there are several gates between this proposal and any
actual deprecation of fromIntegral.
1. At present there is no story here for how users change their code to
address the deprecation. They'd just get to stare at a -Wall full of
warnings and have no migration path.
2. To start a deprecation process here I'd at the very least want to have a
replacement that a decent cross-section of folks have been able to agree is
an improvement. This could be packaged up independently, could be selected
from among other solutions that folks have already packaged up, etc. I don't
care where it comes from so long as you can get enough people agree that it
is the right solution.
3. Build consensus to move it into base. That way to address any eventual
deprecation there can be a road map that doesn't incur new dependencies for
users.
4. Then once it has been around in base long enough that users can migrate
to it in accordance to the "3 Release Policy" then we could build consensus
to perform a deprecation of the existing method.
This is unfortunately a relatively slow and tedious process, but the
alternative is a Prelude-affecting change that breaks our release policies
and all of the books being made with no real notice.
The current usage pattern and policies we have around deprecations isn't to
use them as the first notice of "hey you might want to do something else",
but rather to catch folks on the tail end right before something gets
removed and long after what the user should do to in response has been
carefully documented and considered.
At least it seems to me that this is a large part of the resistance you are
encountering here.
-Edward
Post by Niklas Hambüchen
Hey David,
Post by David Feuer
I'm -1 on deprecating it, and +1 on adding additional conversion
functions of various sorts: explicit widening, explicit narrowing, etc.
What is your reasoning against deprecating it?
That `fromIntegral` is a function that should be used in new code?
That emitting a warning for potentially wrong code is bad by itself?
Or that people have to change their code to be warning free?
To me, a deprecation constitutes an organised, non-disruptive migration
from the old function to the new explicit functions, and I'd really like
to understand what people's reservations against it are.
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Niklas Hambüchen
2017-09-26 18:49:42 UTC
Permalink
Hey Edward and David,
1. [...] no migration path.
2. To start a deprecation process here I'd at the very least want to
have a replacement that a decent cross-section of folks have been able
to agree is an improvement
I agree, as stated before, deprecation without replacement makes no sense.

It's the point of this thread to

* determine what replacements people would consider good
* find if, assuming we have found good replacements, there would be
still other obstacles (such as the ones you mention below)

So far, it seems that there is broad support for more appropriately
named functions while a type-based approach is controversial; I consider
this a useful outcome of this thread, and think that this is also the
approach that can provide the easiest migration path.
4. Then once it has been around in base long enough that users can
migrate to it in accordance to the "3 Release Policy"
<https://prime.haskell.org/wiki/Libraries/3-Release-Policy> then we
could build consensus to perform a deprecation of the existing method.
Waiting for 3 releases seems fine.
But I don't see why consensus could only be built after that.

It seems that if consensus exists already at a time before, deprecation
can be planned ahead of time.
The linked page contains examples of that, e.g. the deprecation of
`Monad.fail` that is already scheduled for 8.6.
The current usage pattern and policies we have around deprecations isn't
to use them as the first notice of "hey you might want to do something
else", but rather to catch folks on the tail end right before something
gets removed and long after what the user should do to in response has
been carefully documented and considered.
That's a good point; are there reasons why we should not use
deprecations for both patterns ("hey you might want to do something
else" and "this is your last notice")?

If yes, maybe we should add means to support the first pattern.

That goes along with another thought:

The fact that the 3-release-policy says that warnings cannot be added
even for -Wall means that currently there is no flag one can use to get
all of the "latest" warnings GHC can produce to make code safer and more
correct.
Perhaps a -Wextra equivalent is needed?

(Personally I find it unfortunate to have a flag called -Wall not emit
all warnings, or it complaining about benign topics like unused imports
while letting data corruption pass unnoticed for long, or an
off-by-default flag intended to produce as many warnings as possible to
be restricted by a backwards compatibility policy, but C compilers at
least make these a common occurrence.)
At least it seems to me that this is a large part of the resistance you
are encountering here.
Thanks for the insights, very appreciated.
David Feuer
2017-09-26 19:46:19 UTC
Permalink
Post by Niklas Hambüchen
It seems that if consensus exists already at a time before, deprecation
can be planned ahead of time.
The linked page contains examples of that, e.g. the deprecation of
`Monad.fail` that is already scheduled for 8.6.
David Luposchainsky drafted the MonadFail proposal in December 2013,
nearly four years ago. But that's not the beginning of the story. In
November 2009, Michael Snoyman wrote, in
https://wiki.haskell.org/index.php?title=Failure&oldid=31465,
Post by Niklas Hambüchen
Fail is referring to the "fail" function, which is part of the Monad typeclass and is almost universally despised.
And that's not the beginning of the story either. By March 2008, Real
World Haskell http://book.realworldhaskell.org/read/monads.html
warning about fail. I don't have the history of the text, but it
currently reads, in a giant warning box,
Post by Niklas Hambüchen
Beware of fail
Many Monad instances don't override the default implementation of fail that we show here, so in those monads, fail uses error. Calling error is usually highly undesirable, since it throws an exception that callers either cannot catch or will not expect. Even if you know that right now you're executing in a monad that has fail do something more sensible, we still recommend avoiding it. It's far too easy to cause yourself a problem later when you refactor your code and forget that a previously safe use of fail might be dangerous in its new context.
The fail function has no business being in the monad class at all, and it should be removed. So I think you should just discourage people from using it in general.
and the next month Cale Gibbard stated
Post by Niklas Hambüchen
[Most] monads can't implement it sensibly, and it's not actually part of the definition of a monad.
The fail function was just a hack to make the do-notation translation as described in the Report "simpler", but it really belongs in a separate typeclass, something more along the lines of the way that Haskell 1.4 did it.
Beginning in February 2008, as
https://stackoverflow.com/questions/8163852/should-i-avoid-using-monad-fail
points out, there was a thread,
http://haskell.1045720.n5.nabble.com/Proposal-Add-Text-Read-maybeRead-Read-a-gt-String-gt-Maybe-a-td3174167.html,
in which several luminaries spoke out, explicitly or implicitly,
against fail being in Monad.

So it seems that there was a fairly clear consensus that having fail
in Monad was a bad idea almost ten years ago, and that it would be
better to put it in a subclass. It's a bit hard to compare that to the
case of fromIntegral, which I think people have generally been only
mildly uncomfortable with, and when we don't know if there is anything
sufficiently better!
Jacques Carette
2017-09-27 22:01:24 UTC
Permalink
And pre-dating all of that was a little noticed paper

http://www.cas.mcmaster.ca/~kahl/Publications/Conf/Kahl-Carette-Ji-2006a.html

with a semantics that shows that "pattern matching" is an effect, and
how to couple that effect with other effects.  It explains the choices
one sees out there 'in the wild' and augments that with many new options.

Jacques

@InProceedings{Kahl-Carette-Ji-2006a,
author = {Wolfram Kahl and Jacques Carette and Xiaoheng Ji},
title = {Bimonadic Semantics for Basic Pattern Matching Calculi},
crossref = {MPC2006},
pages = {253--273}
}

@Proceedings{MPC2006,
title = {Mathematics of Program Construction, {MPC 2006, Kuressaare, Estonia}},
booktitle = {Mathematics of Program Construction, {MPC 2006}},
editor = {Tarmo Uustalu},
year = 2006,
publisher = Springer,
series = LNCS,
volume = {4014},
URL = {http://link.springer.de/link/service/series/0558/tocs/t4014.htm}
}
Post by David Feuer
Post by Niklas Hambüchen
It seems that if consensus exists already at a time before, deprecation
can be planned ahead of time.
The linked page contains examples of that, e.g. the deprecation of
`Monad.fail` that is already scheduled for 8.6.
David Luposchainsky drafted the MonadFail proposal in December 2013,
nearly four years ago. But that's not the beginning of the story. In
November 2009, Michael Snoyman wrote, in
https://wiki.haskell.org/index.php?title=Failure&oldid=31465,
Post by Niklas Hambüchen
Fail is referring to the "fail" function, which is part of the Monad typeclass and is almost universally despised.
And that's not the beginning of the story either. By March 2008, Real
World Haskell http://book.realworldhaskell.org/read/monads.html
warning about fail. I don't have the history of the text, but it
currently reads, in a giant warning box,
Post by Niklas Hambüchen
Beware of fail
Many Monad instances don't override the default implementation of fail that we show here, so in those monads, fail uses error. Calling error is usually highly undesirable, since it throws an exception that callers either cannot catch or will not expect. Even if you know that right now you're executing in a monad that has fail do something more sensible, we still recommend avoiding it. It's far too easy to cause yourself a problem later when you refactor your code and forget that a previously safe use of fail might be dangerous in its new context.
The fail function has no business being in the monad class at all, and it should be removed. So I think you should just discourage people from using it in general.
and the next month Cale Gibbard stated
Post by Niklas Hambüchen
[Most] monads can't implement it sensibly, and it's not actually part of the definition of a monad.
The fail function was just a hack to make the do-notation translation as described in the Report "simpler", but it really belongs in a separate typeclass, something more along the lines of the way that Haskell 1.4 did it.
Beginning in February 2008, as
https://stackoverflow.com/questions/8163852/should-i-avoid-using-monad-fail
points out, there was a thread,
http://haskell.1045720.n5.nabble.com/Proposal-Add-Text-Read-maybeRead-Read-a-gt-String-gt-Maybe-a-td3174167.html,
in which several luminaries spoke out, explicitly or implicitly,
against fail being in Monad.
So it seems that there was a fairly clear consensus that having fail
in Monad was a bad idea almost ten years ago, and that it would be
better to put it in a subclass. It's a bit hard to compare that to the
case of fromIntegral, which I think people have generally been only
mildly uncomfortable with, and when we don't know if there is anything
sufficiently better!
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Niklas Hambüchen
2017-09-27 22:22:27 UTC
Permalink
Post by David Feuer
David Luposchainsky drafted the MonadFail proposal in December 2013,
So it seems that there was a fairly clear consensus that having fail
in Monad was a bad idea almost ten years ago
I'm aware of that and didn't suggest otherwise.
Post by David Feuer
4. Then once it has been around in base [... for 3 releases ...] then
we could build consensus to perform a deprecation of the existing method.

This suggests that the order is:

1. put it in base
2. wait 3 releases
3. build consensus about deprecation

I pointed out (and you did now, too, more elaborately) that in the case
of MonadFail, the consensus about deprecation step (3) was done before
(1) and (2).
Sven Panne
2017-09-28 15:43:31 UTC
Permalink
Post by Niklas Hambüchen
1. put it in base
2. wait 3 releases
3. build consensus about deprecation
[...]
For the current proposal I would really like to see:

0. Put the proposed functions into a separate package

At least for me, it's far from clear what exactly has to be added, how
usable it would be, how many changes are needed in libraries/apps out in
the wild etc., and removing/changing broken things from base is very, very
hard. So it would be wise to let a proposal mature in a separate package.
But that's a general remark for everything which should go into base.

Regarding fromIntegral: I fail to see why this function should be special
in any way. If you use one of the fixed-width integral types, you basically
opt-in into a world of wrap-arounds, silent under-/overflows etc. As
already mentioned in this thread, this doesn't only happen when
fromIntegral is used, it can happen for tons of other operations, too. For
the same reasoning, one would have to deprecate (+), (-), (*) etc. on
fixed-width types.

So in a nutshell: -1 for deprecating fromIntegral, +1 for additional
conversions (but not in base yet).

Bardur Arantsson
2017-09-26 20:56:40 UTC
Permalink
Post by Niklas Hambüchen
Hey Edward and David,
The fact that the 3-release-policy says that warnings cannot be added
even for -Wall means that currently there is no flag one can use to get
all of the "latest" warnings GHC can produce to make code safer and more
correct.
Perhaps a -Wextra equivalent is needed?
This is called -Weverything in Clang-land.

AFAICT it was introduced because it does imply some warnings that are
ridiculously easy to trigger accidentally, and so you don't to run it
along with e.g. -Werror. Obviously you don't want -Werror on anything
you release to the world, but it can be useful to use "-Weverything" +
"-Werror" for local development. The idea is that you get literally
every warning that the compiler developers can think of and then have to
selectively turn off individual warnings. This squares pretty well with
the
'deprecation-before-we-have-replacements-because-you-can-provide-your-own'
narrative, at least.
Post by Niklas Hambüchen
(Personally I find it unfortunate to have a flag called -Wall not emit
all warnings, or it complaining about benign topics like unused imports
while letting data corruption pass unnoticed for long, or an
off-by-default flag intended to produce as many warnings as possible to
be restricted by a backwards compatibility policy, but C compilers at
least make these a common occurrence.)
I think ship has sailed on naming, but a "-Weverything" might be very
useful, I think.

(A "-Wfuture-deprecation" or similar might be warranted too, and should
probably be included in "-Weverything".)

Regards,
Bardur Arantsson
2017-09-27 07:38:36 UTC
Permalink
Post by Bardur Arantsson
I think ship has sailed on naming, but a "-Weverything" might be very
useful, I think.
Sorry, that was a brain fart. There *is* a -Weverything these days.
Post by Bardur Arantsson
(A "-Wfuture-deprecation" or similar might be warranted too, and should
probably be included in "-Weverything".)
This might still be relevant.

Regards,
Niklas Hambüchen
2017-09-27 22:13:37 UTC
Permalink
Post by Bardur Arantsson
The idea is that you get literally
every warning that the compiler developers can think of and then have to
selectively turn off individual warnings. This squares pretty well with
the
'deprecation-before-we-have-replacements-because-you-can-provide-your-own'
narrative, at least.
I feel like "every warning that the compiler developers can think of"
and the motivation we're going after here are pretty far apart.

Warning when one type doesn't fit into another part is not something
that should be in a "low risk warning category".

I was suggesting we might want to have a flag that is free from the
3-release-policy (a backwards-compatibility mechanism); I was not
suggesting to have more verbose warning categories.

In other words, the intent of that would be to be able to subscribe to
warnings that will be enabled in `-Wall` in ~3 years, already at the
current release.
Jon Fairbairn
2017-09-28 08:38:03 UTC
Permalink
Post by Niklas Hambüchen
Post by David Feuer
I'm -1 on deprecating it, and +1 on adding additional conversion
functions of various sorts: explicit widening, explicit narrowing, etc.
What is your reasoning against deprecating it?
May I just interject that fromIntegral itself is not the
problem. There’s nothing wrong with “fromIntegral 5::Ratio
Integer” for example. The problem is having the type “(Num b,
Integral a) => a->b”, which drags in all the horror that is Num.

A type more like “(Integral a, InfinitePrecisionNumber b) => a
-> b” (and an appropriate definition of the class) would allow
its continued use in places where it’s safe.
--
Jón Fairbairn ***@cl.cam.ac.uk
Andrew Martin
2017-09-26 12:06:14 UTC
Permalink
I'm -1 on deprecating this, but I would like to see the docs warn about its
behavior.
Post by Niklas Hambüchen
This is not a joke.
I just found a bug in base (https://ghc.haskell.org/trac/ghc/ticket/14262
import System.IO
hWaitForInput stdin 4294968296
This code should wait for 49.something days, but it waits for 1 second.
It is caused by a quick use of `fromIntegral` to "convert" `Int -> CInt`
(where CInt = Int32), causing an overflow.
I argue that `fromIntegral` causes unnoticed data corruption without any
warning, and thus are worse than partial functions.
In this case, this data corruption occurs in the most fundamental code
that we all use and have to rely upon (`base`).
So I propose that we add a deprecation pragma to `fromIntegral`
(however, keeping it forever, like Java does, as there's no benefit to
removing it, we just want that people use safer functions over time),
and instead provide a `safeFromIntegral` that can only widen ranges, and
one or more `unsafeFromIntegral` or appropriately named functions that
can error, wrap, return Maybe, or have whatever desired behaviour when
the input value doesn't fit into the output type.
http://hackage.haskell.org/package/basement-0.0.2/docs/Basement-From.html
http://hackage.haskell.org/package/basement-0.0.2/docs/
Basement-IntegralConv.html
It is a bit funny how we argue that Haskell is good for high assurance
programming, and we build all sorts of fancy constructs to eliminate
more run-time errors, when in fact we do worse than C in detecting
errors in the most basic types a computer can use, such as converting
between 64-bit and 32-bit integers.
Let's fix that.
It avoids real problems in real production systems that are difficult to
debug when they happen (who writes a unit test checking that a timeout
of 50 days still works? This is the kind of stuff where you have to rely
on -- very basic -- types).
I'd like to probe support for such a change before doing any work on it.
Niklas
_______________________________________________
Libraries mailing list
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
--
-Andrew Thaddeus Martin
Loading...