patch applied (packages/base): Inline Data.Bits.rotate@Int, enables rotate to be constant folded

[Simon Peyton-Jones]

Great stuff Don,

I've learned from experience that in 2 yrs time I will have forgotten why that INLINE is there, and may remove it.  (Probably not in this particular case, but something similar.)  You may think that the info is in the commit message, but in practice I just don't do a darcs annotate to see which patches modified the line, and look at the commit log.

I think it's better to invest effort in putting documentation in the source code.  Rather than clutter up the (Bits Int) instance, nowadays I add
        -- See Note [Constant folding for rotate]
and put a block comment, headed by that same string, lower down in the file.

I have found that this is a robust and effective way of documenting things.  Does that make sense?

I've pushed a patch doing this to Data.Bits.

Maybe I should update the programming conventions wiki!

Simon

| -----Original Message-----
| From: cvs-libraries-bounces at haskell.org [mailto:cvs-libraries-bounces at haskell.org] On Behalf Of Don
| Stewart
| Sent: 02 May 2008 00:10
| To: cvs-libraries at haskell.org
| Subject: patch applied (packages/base): Inline Data.Bits.rotate at Int, enables rotate to be constant
| folded
|
| Thu May  1 16:01:52 PDT 2008  Don Stewart <dons at galois.com>
|   * Inline Data.Bits.rotate at Int, enables rotate to be constant folded
|
|   All other Bits instances seem to inline well enough on their own
|   to enable constant folding, e.g.
|
|       sumU . mapU (`shift` 3) . replicateU 10000000 $ (7 :: Int)
|
|   goes to:
|
|       Main.$wfold =
|         \ (ww_sOb :: Int#) (ww1_sOf :: Int#) ->
|           case ww1_sOf of wild_XM {
|             __DEFAULT -> Main.$wfold (+# ww_sOb 56) (+# wild_XM 1);
|             10000000 -> ww_sOb
|           }
|
|   With this patch, rotate gets inlined and folded too,
|
|       sumU . mapU (`rotate` 3) . replicateU 10000000 $ (7 :: Int)
|
|   to:
|
|     Main.$wfold =
|       \ (ww_sO7 :: Int#) (ww1_sOb :: Int#) ->
|         case ww1_sOb of wild_XM {
|           __DEFAULT -> Main.$wfold (+# ww_sO7 56) (+# wild_XM 1);
|           10000000 -> ww_sO7
|
|   Whereas it was left as a call to $wrotate before.
|
|
|
|     M ./Data/Bits.hs -1 +8
|
| View patch online:
| http://darcs.haskell.org/packages/base/_darcs/patches/20080501230152-cba2c-
| adb8ea1981dbc6427b2d85b054533517ffc3b4c3.gz
| _______________________________________________
| Cvs-libraries mailing list
| Cvs-libraries at haskell.org
| http://www.haskell.org/mailman/listinfo/cvs-libraries