Commit a1dd7dd6 authored by Simon Peyton Jones's avatar Simon Peyton Jones

Fallout from more assiduous RULE warnings

GHC now warns if rules compete, so that it's not predicatable
which will work and which will not. E.g.

{-# RULES
  f (g x) = ...
  g True  = ...
  #-}

If we had (f (g True)) it's not clear which rule would fire.

This showed up fraility in the libraries.

* Suppress warnigns in Control.Arrow, Control.Category for class
  methods. At the moment we simply don't have a good way to write a
  RULE with a class method in the LHS.  See Trac #1595.  Arrow and
  Category attempt to do so; I have silenced the complaints with
  -fno-warn-inline-rule-shadowing, but it's not a great solution.

* Adjust the NOINLINE pragma on 'GHC.Base.map' to account for the
  map/coerce rule

* Adjust the rewrite rules in Enum, especially for the "literal 1"
  case.  See Note [Enum Integer rules for literal 1].

* Suppress warnings for 'bytestring' e.g.
   libraries/bytestring/Data/ByteString.hs:895:1: warning:
      Rule "ByteString specialise break (x==)" may never fire
        because rule "Class op ==" for ‘==’ might fire first
      Probable fix: add phase [n] or [~n] to the competing rule
parent e343c0a7
{-# LANGUAGE Trustworthy #-}
{-# LANGUAGE NoImplicitPrelude #-}
{-# OPTIONS_GHC -fno-warn-inline-rule-shadowing #-}
-- The RULES for the methods of class Arrow may never fire
-- e.g. compose/arr; see Trac #10528
-----------------------------------------------------------------------------
-- |
......
......@@ -2,6 +2,9 @@
{-# LANGUAGE GADTs #-}
{-# LANGUAGE NoImplicitPrelude #-}
{-# LANGUAGE PolyKinds #-}
{-# OPTIONS_GHC -fno-warn-inline-rule-shadowing #-}
-- The RULES for the methods of class Category may never fire
-- e.g. identity/left, identity/right, association; see Trac #10528
-----------------------------------------------------------------------------
-- |
......
......@@ -853,9 +853,10 @@ augment g xs = g (:) xs
-- > map f [x1, x2, ...] == [f x1, f x2, ...]
map :: (a -> b) -> [a] -> [b]
{-# NOINLINE [1] map #-} -- We want the RULE to fire first.
-- It's recursive, so won't inline anyway,
-- but saying so is more explicit
{-# NOINLINE [0] map #-}
-- We want the RULEs "map" and "map/coerce" to fire first.
-- map is recursive, so won't inline anyway,
-- but saying so is more explicit, and silences warnings
map _ [] = []
map f (x:xs) = f x : map f xs
......
......@@ -344,6 +344,7 @@ instance Enum Char where
{-# INLINE enumFromThenTo #-}
enumFromThenTo (C# x1) (C# x2) (C# y) = efdtChar (ord# x1) (ord# x2) (ord# y)
-- See Note [How the Enum rules work]
{-# RULES
"eftChar" [~1] forall x y. eftChar x y = build (\c n -> eftCharFB c n x y)
"efdChar" [~1] forall x1 x2. efdChar x1 x2 = build (\ c n -> efdCharFB c n x1 x2)
......@@ -482,6 +483,13 @@ instance Enum Int where
"eftIntList" [1] eftIntFB (:) [] = eftInt
#-}
{- Note [How the Enum rules work]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* Phase 2: eftInt ---> build . eftIntFB
* Phase 1: inline build; eftIntFB (:) --> eftInt
* Phase 0: optionally inline eftInt
-}
{-# NOINLINE [1] eftInt #-}
eftInt :: Int# -> Int# -> [Int]
-- [x1..x2]
......@@ -510,6 +518,7 @@ eftIntFB c n x0 y | isTrue# (x0 ># y) = n
-- efdInt and efdtInt deal with [a,b..] and [a,b..c].
-- The code is more complicated because of worries about Int overflow.
-- See Note [How the Enum rules work]
{-# RULES
"efdtInt" [~1] forall x1 x2 y.
efdtInt x1 x2 y = build (\ c n -> efdtIntFB c n x1 x2 y)
......@@ -667,13 +676,32 @@ instance Enum Integer where
enumFromTo x lim = enumDeltaToInteger x 1 lim
enumFromThenTo x y lim = enumDeltaToInteger x (y-x) lim
-- See Note [How the Enum rules work]
{-# RULES
"enumDeltaInteger" [~1] forall x y. enumDeltaInteger x y = build (\c _ -> enumDeltaIntegerFB c x y)
"efdtInteger" [~1] forall x y l.enumDeltaToInteger x y l = build (\c n -> enumDeltaToIntegerFB c n x y l)
"enumDeltaInteger" [1] enumDeltaIntegerFB (:) = enumDeltaInteger
"enumDeltaToInteger" [1] enumDeltaToIntegerFB (:) [] = enumDeltaToInteger
"enumDeltaInteger" [~1] forall x y. enumDeltaInteger x y = build (\c _ -> enumDeltaIntegerFB c x y)
"efdtInteger" [~1] forall x d l. enumDeltaToInteger x d l = build (\c n -> enumDeltaToIntegerFB c n x d l)
"efdtInteger1" [~1] forall x l. enumDeltaToInteger x 1 l = build (\c n -> enumDeltaToInteger1FB c n x l)
"enumDeltaToInteger1FB" [1] forall c n x. enumDeltaToIntegerFB c n x 1 = enumDeltaToInteger1FB c n x
"enumDeltaInteger" [1] enumDeltaIntegerFB (:) = enumDeltaInteger
"enumDeltaToInteger" [1] enumDeltaToIntegerFB (:) [] = enumDeltaToInteger
"enumDeltaToInteger1" [1] enumDeltaToInteger1FB (:) [] = enumDeltaToInteger1
#-}
{-
The "1" rules above specialise for the common case where delta = 1,
so that we can avoid the delta>=0 test in enumDeltaToIntegerFB.
Then enumDeltaToInteger1FB is nice and small and can be inlined,
which would allow the constructor to be inlined and good things to happen.
We match on the literal "1" both in phase 2 (rule "efdtInteger1") and
phase 1 (rule "enumDeltaToInteger1FB"), just for belt and braces
We do not do it for Int this way because hand-tuned code already exists, and
the special case varies more from the general case, due to the issue of overflows.
-}
{-# NOINLINE [0] enumDeltaIntegerFB #-}
enumDeltaIntegerFB :: (Integer -> b -> b) -> Integer -> Integer -> b
enumDeltaIntegerFB c x d = x `seq` (x `c` enumDeltaIntegerFB c (x+d) d)
......@@ -693,14 +721,14 @@ enumDeltaToIntegerFB c n x delta lim
| delta >= 0 = up_fb c n x delta lim
| otherwise = dn_fb c n x delta lim
{-# RULES
"enumDeltaToInteger1" [0] forall c n x . enumDeltaToIntegerFB c n x 1 = up_fb c n x 1
#-}
-- This rule ensures that in the common case (delta = 1), we do not do the check here,
-- and also that we have the chance to inline up_fb, which would allow the constructor to be
-- inlined and good things to happen.
-- We do not do it for Int this way because hand-tuned code already exists, and
-- the special case varies more from the general case, due to the issue of overflows.
{-# NOINLINE [0] enumDeltaToInteger1FB #-}
-- Don't inline this until RULE "enumDeltaToInteger" has had a chance to fire
enumDeltaToInteger1FB :: (Integer -> a -> a) -> a
-> Integer -> Integer -> a
enumDeltaToInteger1FB c n x0 lim = go (x0 :: Integer)
where
go x | x > lim = n
| otherwise = x `c` go (x+1)
{-# NOINLINE [1] enumDeltaToInteger #-}
enumDeltaToInteger :: Integer -> Integer -> Integer -> [Integer]
......@@ -708,6 +736,14 @@ enumDeltaToInteger x delta lim
| delta >= 0 = up_list x delta lim
| otherwise = dn_list x delta lim
{-# NOINLINE [1] enumDeltaToInteger1 #-}
enumDeltaToInteger1 :: Integer -> Integer -> [Integer]
-- Special case for Delta = 1
enumDeltaToInteger1 x0 lim = go (x0 :: Integer)
where
go x | x > lim = []
| otherwise = x : go (x+1)
up_fb :: (Integer -> a -> a) -> a -> Integer -> Integer -> Integer -> a
up_fb c n x0 delta lim = go (x0 :: Integer)
where
......
......@@ -37,6 +37,9 @@ utils/hpc_dist-install_EXTRA_HC_OPTS += -fwarn-tabs
######################################################################
# Disable some warnings in packages we use
# Libraries that have dubious RULES
libraries/bytestring_dist-install_EXTRA_HC_OPTS += -fno-warn-inline-rule-shadowing
# Cabal doesn't promise to be warning-free
utils/ghc-cabal_dist_EXTRA_HC_OPTS += -w
libraries/Cabal/Cabal_dist-boot_EXTRA_HC_OPTS += -w
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment