Deprecating fromIntegral

Niklas Hambüchen

21 Sep 2017 21 Sep '17

12:24 p.m.

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... 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

Show replies by date

amindfv＠gmail.com

21 Sep 21 Sep

12:35 p.m.

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

...

El 21 sept 2017, a las 08:24, Niklas Hambüchen escribió:

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...

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 Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Niklas Hambüchen

12:53 p.m.

On 21/09/17 14:35, amindfv@gmail.com wrote:

...

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

1:10 p.m.

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.h... 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/li...) 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. On 21/09/17 14:53, Niklas Hambüchen wrote:

...

On 21/09/17 14:35, amindfv@gmail.com wrote:

...
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

1:21 p.m.

On Thu, 21 Sep 2017, Niklas Hambüchen wrote:

...

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.

...

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

3:34 p.m.

Hey Henning, On 21/09/17 15:21, Henning Thielemann wrote:

...

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.

...

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.)

...

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...

...

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.

...

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.

...

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

3:42 p.m.

On Thu, 21 Sep 2017, Niklas Hambüchen wrote:

...

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

4:02 p.m.

On 21/09/17 17:42, Henning Thielemann wrote:

...

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

22 Sep 22 Sep

12:14 p.m.

...there's one minor detail I'd like to point out as it doesn't seem to have been mentioned so far: On Thu, Sep 21, 2017 at 5:34 PM, Niklas Hambüchen wrote:

...

(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

21 Sep 21 Sep

2:22 p.m.

On 2017-09-21 at 14:24:14 +0200, Niklas Hambüchen wrote: [...]

...

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

[...]

...

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.

...

(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.

[...]

...

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

3:57 p.m.

Hey Herbert,

...

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 This is very useful, thanks for that.

...

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

5:27 p.m.

On Thu, Sep 21, 2017 at 8:57 AM, Niklas Hambüchen wrote:

...

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

11:19 p.m.

On 21/09/17 19:27, Evan Laforge wrote:

...

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-fromintegr... 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

22 Sep 22 Sep

11:30 a.m.

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. On Thu, Sep 21, 2017 at 7:20 PM Niklas Hambüchen wrote:

...

On 21/09/17 19:27, Evan Laforge wrote:

...
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-fromintegr...

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 Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Niklas Hambüchen

12:58 p.m.

On 22/09/17 13:30, Carter Schonwald wrote:

...

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

6:53 p.m.

On Thu, Sep 21, 2017 at 4:19 PM, Niklas Hambüchen wrote:

...

On 21/09/17 19:27, Evan Laforge wrote:

...
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 languages. But, as shown below, my guesses can be pretty wrong:

...

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.

...

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

23 Sep 23 Sep

9:28 p.m.

On Fri, 22 Sep 2017, Evan Laforge wrote:

...

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

9:31 p.m.

On Fri, 22 Sep 2017, Evan Laforge wrote:

...

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

21 Sep 21 Sep

5:47 p.m.

...

...
...
...
...
"NH" == Niklas Hambüchen writes:

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

9:30 p.m.

On 21/09/17 19:47, John Wiegley wrote:

...

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

10:01 p.m.

...

...
...
...
...
"NH" == Niklas Hambüchen writes:

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

10:12 p.m.

...

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")

...

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

10:24 p.m.

...

...
...
...
...
"NH" == Niklas Hambüchen writes:

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

amindfv＠gmail.com

22 Sep 22 Sep

11:07 p.m.

El 21 sept 2017, a las 18:01, John Wiegley escribió:

...

...
...
...
...
...
"NH" == Niklas Hambüchen writes:

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

...

-- John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2 _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Edward Kmett

21 Sep 21 Sep

2:41 p.m.

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 On Thu, Sep 21, 2017 at 8:24 AM, Niklas Hambüchen wrote:

...

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 _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Niklas Hambüchen

5:26 p.m.

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

5:57 p.m.

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 On Thu, Sep 21, 2017 at 1:26 PM, Niklas Hambüchen wrote:

...

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

Niklas Hambüchen

10:02 p.m.

On 21/09/17 19:57, Edward Kmett wrote:

...

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).

...

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

David Feuer

25 Sep 25 Sep

9:35 p.m.

I'm -1 on deprecating it, and +1 on adding additional conversion functions of various sorts: explicit widening, explicit narrowing, etc. On Sep 21, 2017 8:25 AM, "Niklas Hambüchen" wrote:

...

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 _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Niklas Hambüchen

26 Sep 26 Sep

3:08 p.m.

Hey David,

...

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

4:41 p.m.

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 On Tue, Sep 26, 2017 at 11:08 AM, Niklas Hambüchen wrote:

...

Hey David,

...
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 Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

David Feuer

5 p.m.

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. On Tue, Sep 26, 2017 at 12:41 PM, Edward Kmett wrote:

...

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

On Tue, Sep 26, 2017 at 11:08 AM, Niklas Hambüchen wrote:

...
Hey David,

...
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 Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Niklas Hambüchen

6:49 p.m.

Hey Edward and David, Edward Kmett wrote:

...

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

7:46 p.m.

On Tue, Sep 26, 2017 at 2:49 PM, Niklas Hambüchen wrote:

...

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,

...

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,

...

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.

In a comment on the text in March 2008, Lennart Augustssen wrote

...

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

...

[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..., 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

27 Sep 27 Sep

10:01 p.m.

New subject: Monad.fail, was Re: Deprecating fromIntegral

And pre-dating all of that was a little noticed paper http://www.cas.mcmaster.ca/~kahl/Publications/Conf/Kahl-Carette-Ji-2006a.htm... 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} } On 2017-09-26 03:46 PM, David Feuer wrote:

...

On Tue, Sep 26, 2017 at 2:49 PM, Niklas Hambüchen wrote:

...
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,

...
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,

...
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. In a comment on the text in March 2008, Lennart Augustssen wrote

...
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

...
[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..., 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 Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Niklas Hambüchen

10:22 p.m.

On 26/09/17 21:46, David Feuer wrote:

...

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. I was replying specifically to:

...

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

28 Sep 28 Sep

3:43 p.m.

2017-09-28 0:22 GMT+02:00 Niklas Hambüchen :

...

This suggests that the order is:

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

26 Sep 26 Sep

8:56 p.m.

On 2017-09-26 20:49, Niklas Hambüchen wrote:

...

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.

...

(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

27 Sep 27 Sep

7:38 a.m.

On 2017-09-26 22:56, Bardur Arantsson wrote:

...

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.

...

(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

10:13 p.m.

On 26/09/17 22:56, Bardur Arantsson wrote:

...

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

28 Sep 28 Sep

8:38 a.m.

Niklas Hambüchen writes:

...

...
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 Jon.Fairbairn@cl.cam.ac.uk

Andrew Martin

26 Sep 26 Sep

12:06 p.m.

I'm -1 on deprecating this, but I would like to see the docs warn about its behavior. On Thu, Sep 21, 2017 at 8:24 AM, Niklas Hambüchen wrote:

...

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 _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

-- -Andrew Thaddeus Martin

Niklas Hambüchen

8 Aug 8 Aug

3:08 a.m.

Today I found another big bug caused by `fromIntegral`: https://github.com/haskell-crypto/cryptonite/issues/330 Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA hash collisions in my productions system. Restating what I said there: * Until we deprecate fromIntegral, Haskell code will always be subtly wrong and never be secure. * If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps. The wrong code, hashInternalUpdate ctx d (fromIntegral $ B.length b) exists because it simply does not look like wrong code. In contrast, hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b) does look like wrong code and would make anyone scrolling by suspicious. We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one.

Herbert Valerio Riedel

6:19 a.m.

Btw, subtle correctness issues hiding throughout the vast monolithic codebase like these were part of the reason (other reasons are explained in https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul...) why cryptohash/cryptonite/foundation has been banned from our codebases not the least because it's hopeless to get something like cryptonite through formal certification for security critical applications. At the time the author made it clear he didn't welcome my bug reports (later I learned the author was merely peculiar as to what they consider an actual bug worth reporting). And whenever I mentioned this in the past on reddit as a PSA, I got defensive reactions and disbelief from certain people who were already invested in the cryptonite ecosystem and I was accused of spreading unfounded FUD... and so I stopped bringing up this kind of "heresy" publicly. However, as you can see in https://hackage.haskell.org/package/cryptohash-sha256/changelog this particular 32-bit overflow issue was one of the critical bugfixes I ran into and repaired in the very first release right after the initial-fork-release back in 2016. It's astonishing it took over 4 years for this bug to keep lingering in cryptonite/cryptohash before it was detected. You'd expect this to have been detected in code audits performed by Haskell companies that actively promote the use of cryptonite early on. On Sat, Aug 8, 2020 at 5:09 AM Niklas Hambüchen via Libraries wrote:

...

Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA hash collisions in my productions system.

Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly wrong and never be secure. * If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Carter Schonwald

10 Aug 10 Aug

3:43 a.m.

The real issue here isn’t fromintrgral, but that everything defaults to wrapping semantics Adding variants that do signaling/exceptions and clipping Variants Of finite word/Int data types rather than just wrapping is the right path forward. On Sat, Aug 8, 2020 at 2:20 AM Herbert Valerio Riedel wrote:

...

Btw, subtle correctness issues hiding throughout the vast monolithic codebase like these were part of the reason (other reasons are explained in https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul... ) why cryptohash/cryptonite/foundation has been banned from our codebases not the least because it's hopeless to get something like cryptonite through formal certification for security critical applications. At the time the author made it clear he didn't welcome my bug reports (later I learned the author was merely peculiar as to what they consider an actual bug worth reporting). And whenever I mentioned this in the past on reddit as a PSA, I got defensive reactions and disbelief from certain people who were already invested in the cryptonite ecosystem and I was accused of spreading unfounded FUD... and so I stopped bringing up this kind of "heresy" publicly.

However, as you can see in

https://hackage.haskell.org/package/cryptohash-sha256/changelog

this particular 32-bit overflow issue was one of the critical bugfixes I ran into and repaired in the very first release right after the initial-fork-release back in 2016. It's astonishing it took over 4 years for this bug to keep lingering in cryptonite/cryptohash before it was detected. You'd expect this to have been detected in code audits performed by Haskell companies that actively promote the use of cryptonite early on.

On Sat, Aug 8, 2020 at 5:09 AM Niklas Hambüchen via Libraries wrote:

...
Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA hash

collisions in my productions system.

...
Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly

wrong and never be secure.

...
* If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Carter Schonwald

4:22 a.m.

I mean Saturating not clipping On Sun, Aug 9, 2020 at 11:43 PM Carter Schonwald wrote:

...

The real issue here isn’t fromintrgral, but that everything defaults to wrapping semantics

Adding variants that do signaling/exceptions and clipping Variants Of finite word/Int data types rather than just wrapping is the right path forward.

On Sat, Aug 8, 2020 at 2:20 AM Herbert Valerio Riedel wrote:

...
Btw, subtle correctness issues hiding throughout the vast monolithic codebase like these were part of the reason (other reasons are explained in https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul... ) why cryptohash/cryptonite/foundation has been banned from our codebases not the least because it's hopeless to get something like cryptonite through formal certification for security critical applications. At the time the author made it clear he didn't welcome my bug reports (later I learned the author was merely peculiar as to what they consider an actual bug worth reporting). And whenever I mentioned this in the past on reddit as a PSA, I got defensive reactions and disbelief from certain people who were already invested in the cryptonite ecosystem and I was accused of spreading unfounded FUD... and so I stopped bringing up this kind of "heresy" publicly.

However, as you can see in

https://hackage.haskell.org/package/cryptohash-sha256/changelog

this particular 32-bit overflow issue was one of the critical bugfixes I ran into and repaired in the very first release right after the initial-fork-release back in 2016. It's astonishing it took over 4 years for this bug to keep lingering in cryptonite/cryptohash before it was detected. You'd expect this to have been detected in code audits performed by Haskell companies that actively promote the use of cryptonite early on.

On Sat, Aug 8, 2020 at 5:09 AM Niklas Hambüchen via Libraries wrote:

...
Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA

hash collisions in my productions system.

...
Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly

wrong and never be secure.

...
* If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

David Feuer

5:20 a.m.

There isn't one problem. Different situations require different things. Wrapping makes fromInteger a ring homomorphism, which makes a lot of basic arithmetic intuition "just work". But fromIntegral is a strange and lawless beast, as well as a nasty potential optimization pitfall. On Sun, Aug 9, 2020, 11:44 PM Carter Schonwald wrote:

...

The real issue here isn’t fromintrgral, but that everything defaults to wrapping semantics

Adding variants that do signaling/exceptions and clipping Variants Of finite word/Int data types rather than just wrapping is the right path forward.

On Sat, Aug 8, 2020 at 2:20 AM Herbert Valerio Riedel wrote:

...
Btw, subtle correctness issues hiding throughout the vast monolithic codebase like these were part of the reason (other reasons are explained in https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul... ) why cryptohash/cryptonite/foundation has been banned from our codebases not the least because it's hopeless to get something like cryptonite through formal certification for security critical applications. At the time the author made it clear he didn't welcome my bug reports (later I learned the author was merely peculiar as to what they consider an actual bug worth reporting). And whenever I mentioned this in the past on reddit as a PSA, I got defensive reactions and disbelief from certain people who were already invested in the cryptonite ecosystem and I was accused of spreading unfounded FUD... and so I stopped bringing up this kind of "heresy" publicly.

However, as you can see in

https://hackage.haskell.org/package/cryptohash-sha256/changelog

this particular 32-bit overflow issue was one of the critical bugfixes I ran into and repaired in the very first release right after the initial-fork-release back in 2016. It's astonishing it took over 4 years for this bug to keep lingering in cryptonite/cryptohash before it was detected. You'd expect this to have been detected in code audits performed by Haskell companies that actively promote the use of cryptonite early on.

On Sat, Aug 8, 2020 at 5:09 AM Niklas Hambüchen via Libraries wrote:

...
Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA

hash collisions in my productions system.

...
Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly

wrong and never be secure.

...
* If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Vanessa McHale

11:10 a.m.

I agree! Silently changing semantics under one’s feet may be bad, but one needn’t remove the function itself. That’s just work for library maintainers! Cheers, Vanessa McHale

...

On Aug 9, 2020, at 10:43 PM, Carter Schonwald wrote:

The real issue here isn’t fromintrgral, but that everything defaults to wrapping semantics

Adding variants that do signaling/exceptions and clipping Variants Of finite word/Int data types rather than just wrapping is the right path forward.

On Sat, Aug 8, 2020 at 2:20 AM Herbert Valerio Riedel mailto:hvriedel@gmail.com> wrote: Btw, subtle correctness issues hiding throughout the vast monolithic codebase like these were part of the reason (other reasons are explained in https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul... https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul...) why cryptohash/cryptonite/foundation has been banned from our codebases not the least because it's hopeless to get something like cryptonite through formal certification for security critical applications. At the time the author made it clear he didn't welcome my bug reports (later I learned the author was merely peculiar as to what they consider an actual bug worth reporting). And whenever I mentioned this in the past on reddit as a PSA, I got defensive reactions and disbelief from certain people who were already invested in the cryptonite ecosystem and I was accused of spreading unfounded FUD... and so I stopped bringing up this kind of "heresy" publicly.

However, as you can see in

https://hackage.haskell.org/package/cryptohash-sha256/changelog https://hackage.haskell.org/package/cryptohash-sha256/changelog

this particular 32-bit overflow issue was one of the critical bugfixes I ran into and repaired in the very first release right after the initial-fork-release back in 2016. It's astonishing it took over 4 years for this bug to keep lingering in cryptonite/cryptohash before it was detected. You'd expect this to have been detected in code audits performed by Haskell companies that actively promote the use of cryptonite early on.

On Sat, Aug 8, 2020 at 5:09 AM Niklas Hambüchen via Libraries mailto:libraries@haskell.org> wrote:

...
Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330 https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA hash collisions in my productions system.

Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly wrong and never be secure. * If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org mailto:Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org mailto:Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Carter Schonwald

1:49 p.m.

Agreed! And 1-2 maintainers egregious bugs isn’t evidence enough for baby and bath water. As david and others say, there’s a lot of different semantics for mapping integers to finite words and groups etc, and the issue here is ultimately a bad choice in target. Int32 is never a portable size for byte arrays. In c or Haskell. On Mon, Aug 10, 2020 at 7:10 AM Vanessa McHale wrote:

...

I agree! Silently changing semantics under one’s feet may be bad, but one needn’t remove the function itself. That’s just work for library maintainers!

Cheers, Vanessa McHale

On Aug 9, 2020, at 10:43 PM, Carter Schonwald wrote:

The real issue here isn’t fromintrgral, but that everything defaults to wrapping semantics

Adding variants that do signaling/exceptions and clipping Variants Of finite word/Int data types rather than just wrapping is the right path forward.

On Sat, Aug 8, 2020 at 2:20 AM Herbert Valerio Riedel wrote:

...
Btw, subtle correctness issues hiding throughout the vast monolithic codebase like these were part of the reason (other reasons are explained in https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul... ) why cryptohash/cryptonite/foundation has been banned from our codebases not the least because it's hopeless to get something like cryptonite through formal certification for security critical applications. At the time the author made it clear he didn't welcome my bug reports (later I learned the author was merely peculiar as to what they consider an actual bug worth reporting). And whenever I mentioned this in the past on reddit as a PSA, I got defensive reactions and disbelief from certain people who were already invested in the cryptonite ecosystem and I was accused of spreading unfounded FUD... and so I stopped bringing up this kind of "heresy" publicly.

However, as you can see in

https://hackage.haskell.org/package/cryptohash-sha256/changelog

this particular 32-bit overflow issue was one of the critical bugfixes I ran into and repaired in the very first release right after the initial-fork-release back in 2016. It's astonishing it took over 4 years for this bug to keep lingering in cryptonite/cryptohash before it was detected. You'd expect this to have been detected in code audits performed by Haskell companies that actively promote the use of cryptonite early on.

On Sat, Aug 8, 2020 at 5:09 AM Niklas Hambüchen via Libraries wrote:

...
Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA

hash collisions in my productions system.

...
Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly

wrong and never be secure.

...
* If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Eric Mertens

4:11 p.m.

Step one is to build the new preferred API and then step two is to worry about moving the whole ecosystem over. I’d love to have some safer numeric conversion functions around. Is there a good starting point already or do we need to build something new?

...

On Aug 10, 2020, at 6:49 AM, Carter Schonwald wrote:

Agreed!

And 1-2 maintainers egregious bugs isn’t evidence enough for baby and bath water.

As david and others say, there’s a lot of different semantics for mapping integers to finite words and groups etc, and the issue here is ultimately a bad choice in target. Int32 is never a portable size for byte arrays. In c or Haskell.

On Mon, Aug 10, 2020 at 7:10 AM Vanessa McHale mailto:vamchale@gmail.com> wrote: I agree! Silently changing semantics under one’s feet may be bad, but one needn’t remove the function itself. That’s just work for library maintainers!

Cheers, Vanessa McHale

...
On Aug 9, 2020, at 10:43 PM, Carter Schonwald mailto:carter.schonwald@gmail.com> wrote:

The real issue here isn’t fromintrgral, but that everything defaults to wrapping semantics

Adding variants that do signaling/exceptions and clipping Variants Of finite word/Int data types rather than just wrapping is the right path forward.

On Sat, Aug 8, 2020 at 2:20 AM Herbert Valerio Riedel mailto:hvriedel@gmail.com> wrote: Btw, subtle correctness issues hiding throughout the vast monolithic codebase like these were part of the reason (other reasons are explained in https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul... https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul...) why cryptohash/cryptonite/foundation has been banned from our codebases not the least because it's hopeless to get something like cryptonite through formal certification for security critical applications. At the time the author made it clear he didn't welcome my bug reports (later I learned the author was merely peculiar as to what they consider an actual bug worth reporting). And whenever I mentioned this in the past on reddit as a PSA, I got defensive reactions and disbelief from certain people who were already invested in the cryptonite ecosystem and I was accused of spreading unfounded FUD... and so I stopped bringing up this kind of "heresy" publicly.

However, as you can see in

https://hackage.haskell.org/package/cryptohash-sha256/changelog https://hackage.haskell.org/package/cryptohash-sha256/changelog

this particular 32-bit overflow issue was one of the critical bugfixes I ran into and repaired in the very first release right after the initial-fork-release back in 2016. It's astonishing it took over 4 years for this bug to keep lingering in cryptonite/cryptohash before it was detected. You'd expect this to have been detected in code audits performed by Haskell companies that actively promote the use of cryptonite early on.

On Sat, Aug 8, 2020 at 5:09 AM Niklas Hambüchen via Libraries mailto:libraries@haskell.org> wrote:

...
Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330 https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA hash collisions in my productions system.

Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly wrong and never be secure. * If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org mailto:Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org mailto:Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries _______________________________________________ Libraries mailing list Libraries@haskell.org mailto:Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Carter Schonwald

5:33 p.m.

well said. On Mon, Aug 10, 2020 at 12:12 PM Eric Mertens wrote:

...

Step one is to build the new preferred API and then step two is to worry about moving the whole ecosystem over. I’d love to have some safer numeric conversion functions around. Is there a good starting point already or do we need to build something new?

On Aug 10, 2020, at 6:49 AM, Carter Schonwald wrote:

Agreed!

And 1-2 maintainers egregious bugs isn’t evidence enough for baby and bath water.

As david and others say, there’s a lot of different semantics for mapping integers to finite words and groups etc, and the issue here is ultimately a bad choice in target. Int32 is never a portable size for byte arrays. In c or Haskell.

On Mon, Aug 10, 2020 at 7:10 AM Vanessa McHale wrote:

...
I agree! Silently changing semantics under one’s feet may be bad, but one needn’t remove the function itself. That’s just work for library maintainers!

Cheers, Vanessa McHale

On Aug 9, 2020, at 10:43 PM, Carter Schonwald wrote:

The real issue here isn’t fromintrgral, but that everything defaults to wrapping semantics

Adding variants that do signaling/exceptions and clipping Variants Of finite word/Int data types rather than just wrapping is the right path forward.

On Sat, Aug 8, 2020 at 2:20 AM Herbert Valerio Riedel wrote:

...
Btw, subtle correctness issues hiding throughout the vast monolithic codebase like these were part of the reason (other reasons are explained in https://old.reddit.com/r/haskell/comments/5lxv75/psa_please_use_unique_modul... ) why cryptohash/cryptonite/foundation has been banned from our codebases not the least because it's hopeless to get something like cryptonite through formal certification for security critical applications. At the time the author made it clear he didn't welcome my bug reports (later I learned the author was merely peculiar as to what they consider an actual bug worth reporting). And whenever I mentioned this in the past on reddit as a PSA, I got defensive reactions and disbelief from certain people who were already invested in the cryptonite ecosystem and I was accused of spreading unfounded FUD... and so I stopped bringing up this kind of "heresy" publicly.

However, as you can see in

https://hackage.haskell.org/package/cryptohash-sha256/changelog

this particular 32-bit overflow issue was one of the critical bugfixes I ran into and repaired in the very first release right after the initial-fork-release back in 2016. It's astonishing it took over 4 years for this bug to keep lingering in cryptonite/cryptohash before it was detected. You'd expect this to have been detected in code audits performed by Haskell companies that actively promote the use of cryptonite early on.

On Sat, Aug 8, 2020 at 5:09 AM Niklas Hambüchen via Libraries wrote:

...
Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA

hash collisions in my productions system.

...
Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly

wrong and never be secure.

...
* If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Vanessa McHale

8 Aug 8 Aug

1:44 p.m.

-1 from me, massive work to overhaul the ecosystem. Maybe a haddock comment first?

...

On Aug 7, 2020, at 10:08 PM, Niklas Hambüchen via Libraries wrote:

Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA hash collisions in my productions system.

Restating what I said there:

* Until we deprecate fromIntegral, Haskell code will always be subtly wrong and never be secure. * If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps.

The wrong code,

hashInternalUpdate ctx d (fromIntegral $ B.length b)

exists because it simply does not look like wrong code. In contrast,

hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b)

does look like wrong code and would make anyone scrolling by suspicious.

We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. _______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Elliot Cameron

2:09 p.m.

I agree the situation now is actually quite dire. It would be much, much better to be partial than to corrupt data. Sadly even something as ubiquitous as + and - suffer from a similar problem. The issue is in many ways polymorphism. These functions are far less polymorphic than their signatures. What if we introduced partial versions of these functions and deprecated the rest. Then if someone wants to guarantee there are no exceptions they need to resort to more advanced tactics but at the very least we don't corrupt. On Sat, Aug 8, 2020, 9:45 AM Vanessa McHale wrote:

...

-1 from me, massive work to overhaul the ecosystem.

Maybe a haddock comment first?

...
On Aug 7, 2020, at 10:08 PM, Niklas Hambüchen via Libraries < libraries@haskell.org> wrote:

Today I found another big bug caused by `fromIntegral`:

https://github.com/haskell-crypto/cryptonite/issues/330

Incorrect hashes for all hash algorithms beyond 4 GiB of input. SHA hash

collisions in my productions system. > > Restating what I said there: > > * Until we deprecate fromIntegral, Haskell code will always be subtly wrong and never be secure. > * If we don't fix this, people will shy away from using Haskell for serious work (or learn it the hard way). Rust and C both do this better. > * If the authors of key crypto libraries fall for these traps (no blame on them), who can get it right? We should remove the traps. > > The wrong code, > > hashInternalUpdate ctx d (fromIntegral $ B.length b) > > exists because it simply does not look like wrong code. In contrast, > > hashInternalUpdate ctx d (fromIntegralWrapping $ B.length b) > > does look like wrong code and would make anyone scrolling by suspicious. > > We can look away while continuing to claim that Haskell is a high-correctness language, or fix stuff like this and make it one. > _______________________________________________ > Libraries mailing list > Libraries@haskell.org > http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Henning Thielemann

2:51 p.m.

On Sat, 8 Aug 2020, Vanessa McHale wrote:

...

-1 from me, massive work to overhaul the ecosystem.

Maybe a haddock comment first?

Maybe a separate library to solve these numeric problems the correct way and then ban every use of Prelude or 'base' in security related packages and then move on to other packages?

Bardur Arantsson

10 Aug 10 Aug

7:13 a.m.

On 08/08/2020 15.44, Vanessa McHale wrote:

...

-1 from me, massive work to overhaul the ecosystem.

Why would a massive overhaul be necessary for deprecation? If that's the case then there's a deeper more serious underlying issue around deprecation, IMO. Regards,

Sven Panne

8:34 a.m.

Am Mo., 10. Aug. 2020 um 09:15 Uhr schrieb Bardur Arantsson < spam@scientician.net>:

...

On 08/08/2020 15.44, Vanessa McHale wrote:

...
-1 from me, massive work to overhaul the ecosystem.

Why would a massive overhaul be necessary for deprecation? If that's the case then there's a deeper more serious underlying issue around deprecation, IMO.

Two things come to my mind here: * You'll probably break quite a few projects which use -Werror. I know that there are different opinions regarding -Werror in general, but in any case there *will* be breakage. * If you consider books and tutorials a part of the ecosystem (which I definitely do), there is even more "breakage": From the top of my head I would say that quite a few of them use fromIntegral, so deprecation will cause confusion. All these things are definitely fixable, but neither quickly nor without a negligible cost. Deprecations should not be done lightly. Regarding the deprecation itself: I fail to see why fromIntegral is worse than (+), (-), (*), ..., and nobody is proposing to remove these. The real problem is using fixed-sized numbers where they shouldn't be used, so a -1 from me. Cheers, S.

David Feuer

8:44 a.m.

I wonder if this is something HLint could help with. I imagine many bugs could be avoided if every use of fromIntegral used visible type application to indicate explicitly what type was being converted to what type. On Mon, Aug 10, 2020, 4:34 AM Sven Panne wrote:

...

Am Mo., 10. Aug. 2020 um 09:15 Uhr schrieb Bardur Arantsson < spam@scientician.net>:

...
On 08/08/2020 15.44, Vanessa McHale wrote:

...
-1 from me, massive work to overhaul the ecosystem.

Why would a massive overhaul be necessary for deprecation? If that's the case then there's a deeper more serious underlying issue around deprecation, IMO.

Two things come to my mind here:

* You'll probably break quite a few projects which use -Werror. I know that there are different opinions regarding -Werror in general, but in any case there *will* be breakage.

* If you consider books and tutorials a part of the ecosystem (which I definitely do), there is even more "breakage": From the top of my head I would say that quite a few of them use fromIntegral, so deprecation will cause confusion.

All these things are definitely fixable, but neither quickly nor without a negligible cost. Deprecations should not be done lightly.

Regarding the deprecation itself: I fail to see why fromIntegral is worse than (+), (-), (*), ..., and nobody is proposing to remove these. The real problem is using fixed-sized numbers where they shouldn't be used, so a -1 from me.

Cheers, S.

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

Sakumatti Luukkonen

9:06 a.m.

I imagine that STAN (https://github.com/kowainik/stan) should be able to detect dangerous uses of fromIntegral even without visible type applications. But perhaps GHC could simply issue a warning in such cases? ma 10.8.2020 klo 11.45 David Feuer kirjoitti:

...

I wonder if this is something HLint could help with. I imagine many bugs could be avoided if every use of fromIntegral used visible type application to indicate explicitly what type was being converted to what type.

On Mon, Aug 10, 2020, 4:34 AM Sven Panne wrote:

...
Am Mo., 10. Aug. 2020 um 09:15 Uhr schrieb Bardur Arantsson < spam@scientician.net>:

...
On 08/08/2020 15.44, Vanessa McHale wrote:

...
-1 from me, massive work to overhaul the ecosystem.

Why would a massive overhaul be necessary for deprecation? If that's the case then there's a deeper more serious underlying issue around deprecation, IMO.

Two things come to my mind here:

* You'll probably break quite a few projects which use -Werror. I know that there are different opinions regarding -Werror in general, but in any case there *will* be breakage.

* If you consider books and tutorials a part of the ecosystem (which I definitely do), there is even more "breakage": From the top of my head I would say that quite a few of them use fromIntegral, so deprecation will cause confusion.

All these things are definitely fixable, but neither quickly nor without a negligible cost. Deprecations should not be done lightly.

Regarding the deprecation itself: I fail to see why fromIntegral is worse than (+), (-), (*), ..., and nobody is proposing to remove these. The real problem is using fixed-sized numbers where they shouldn't be used, so a -1 from me.

Cheers, S.

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries

-- Sakumatti Luukkonen

Edward Kmett

12 Aug 12 Aug

6 p.m.

On Mon, Aug 10, 2020 at 12:15 AM Bardur Arantsson wrote:

...

Why would a massive overhaul be necessary for deprecation? If that's the case then there's a deeper more serious underlying issue around deprecation, IMO.

Deprecation would at a minimum first require us to offer an alternative, let that percolate through code that uses base and only then apply the deprecation pragma under the 3 release policy. Otherwise there isn't a path for users to write code that compiles without warnings without pragmas. Keep in mind fromIntegral is a Haskell Report facing change, so it is the sort of thing that the CLC tends to hold to a higher bar still. Changes to it will per force invalidate a lot of training material. I'm not really weighing in on if this is a good or a bad change with that, just that there is a non-trivial amount of process to enacting it if the community does decide to move forward. That isn't "a serious underlying issue", so much as establishing a baseline of stability and usability. I'm personally +1 on the *idea* that it'd be good to find a solution that allows you to safely identify whether you want a coercion that can truncate or not. The issue re-raised is a good one. But I don't happen to like the solution that was offered when the corpse of this issue was disinterred. Now for where I think this proposal at least insofar as it references a concrete plan of attack falls down, fromIntegral leans on toInteger and fromInteger as a common intermediary. This allows O(n) instances where the modules linked by Niklas would require O(n^2). But it is even worse than that. As a library author I don't need to know your integral type to be able to fromIntegral from mine to yours today, but I really would in a world where that became unavailable. Both of the modules linked use fundep-less multi-parameter typeclasses, which means type inference for them is terrible, (and even in the single parameter type class case we have, remember, with Num, defaulting can kick in, we're even farther removed from such a safety net here) and instances will easily accidentally overlap the moment you go to define something like From a (Forward a), making this problem even worse for any non-trivial numeric type. So in light of that I'm personally strongly -1 on the concrete set of actions being proposed untless a decent API can be found that scales, doesn't have those problems, and could be written into something like a Haskell Report without dipping into language extensions we haven't formalized in a Report. That isn't to say there isn't some variant of a proposal that can't be found, but I'm having a hard time satisfying all the constraints that a Prelude facing change really should meet. Now, that isn't to say something can't be done, for instance, weaker compromises like providing a module in base with safer casts and the like could perhaps use whatever language extensions were suited to the task, as there you have a lot more freedom to use more modern Haskell. But even there I'd still like to see a way that factors the O(n^2) cases into O(n) and which doesn't block any decently polymorphic numeric types behind overlapping instances and don't make type inference go to hell. So, let's see if we can't find a proposal that doesn't violate the gauntlet of constraints imposed above. Off the cuff: Add a variant of fromInteger to Num that returns Maybe a. class Num a where ... fromIntegerMaybe :: Integer -> Maybe a fromIntegerMaybe a = Just (fromInteger a) Modify the existing instances of Num to implement this extra member, and having it return Nothing if the Integer is out of bounds. As a concrete point in the proposal design space, keep the existing fromInteger/fromIntegral as a wrapping conversion. Why? It dodges the Haskell Report change. Others might disagree. I'm just trying to offer fuel for the debate that takes it in a productive direction. fromIntegralMaybe :: (Integral a, Num b) => a -> Maybe b fromIntegralMaybe = fromIntegerMaybe . toInteger can now be used for safe conversions. There are 3 target semantics one might reasonably want to see in their code: 1.) Wrapping (existing fromIntegral) 2.) throwing an exception on overflow (via some wrapping combinator that just handles the above Maybe) 3.) Return Nothing so the error can be handled in pure code. Each can be build on top of fromIntegral and fromIntegralMaybe. Room for variation: * fromIntegralMaybe could be switched to something like Integer -> Either String a, which would let you give back an error message saying why you didn't like the Integer. * fromIntegralWrapped could be added explicitly, and then after a suitable period fromIntegral could be deprecated, but this would require an annoying dance where users would have to switch what they define, which is non-trivial to the point of ner impossibility under the 3 release policy, so I'd personally not go that way, but hey, it is not my call. * Shorter names might be nice. fromIntegral is long enough that we get mocked by other language communities. Adding more words to the combinator name here compounds that issue. I mention this because something along these lines would address the substance of the issue here without inducing the horrible unusability that I feel the concrete proposal offered here would create. -Edward

1789

Age (days ago)

2845

Last active (days ago)

List overview

Download

58 comments

18 participants

participants (18)

amindfv＠gmail.com
Andrew Martin
Bardur Arantsson
Carter Schonwald
David Feuer
Edward Kmett
Elliot Cameron
Eric Mertens
Evan Laforge
Henning Thielemann
Herbert Valerio Riedel
Jacques Carette
John Wiegley
Jon Fairbairn
Niklas Hambüchen
Sakumatti Luukkonen
Sven Panne
Vanessa McHale