make iToBase62's inner loop stricter in one of its arguments
authorAlp Mestanogullari <alp@well-typed.com>
Sun, 2 Sep 2018 20:06:55 +0000 (22:06 +0200)
committerKrzysztof Gogolewski <krz.gogolewski@gmail.com>
Sun, 2 Sep 2018 20:06:56 +0000 (22:06 +0200)
Summary:
hadrian's support for dynamic ways is currently broken (see hadrian#641 [1]).
The stage 1 GHCs that hadrian produces end up producing bad code for
the `iToBase62` function after a few optimisation passes.

In the case where `quotRem` returns (overflowError, 0),
GHC isn't careful enough to realise q is _|_ and happily inlines,
distributes and floats code around until we end up trying to access
index `minBound :: Int` of an array of 62 chars, as a result of inlining
the definition of `quotRem` for Ints, in particular the minBound branch [2].

I will separately look into reproducing the bad transformation on a small
self-contained example and filling a ticket.

[1]: https://github.com/snowleopard/hadrian/issues/641
[2]: https://git.haskell.org/ghc.git/blob/HEAD:/libraries/base/GHC/Real.hs#l366

Test Plan: fixes hadrian#641

Reviewers: bgamari, tdammers

Reviewed By: tdammers

Subscribers: tdammers, rwbarton, carter

Differential Revision: https://phabricator.haskell.org/D5106

compiler/basicTypes/Unique.hs

index 4a709d2..b5c0fce 100644 (file)
@@ -325,7 +325,7 @@ iToBase62 n_
     go n cs | n < 62
             = let !c = chooseChar62 n in c : cs
             | otherwise
-            = go q (c : cs) where (q, r) = quotRem n 62
+            = go q (c : cs) where (!q, r) = quotRem n 62
                                   !c = chooseChar62 r
 
     chooseChar62 :: Int -> Char