Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#4488 closed feature request (fixed)

Warn about unnecessary fromIntegral and other conversions

Reported by: mitar Owned by:
Priority: normal Milestone: 7.4.1
Component: Compiler Version: 6.12.3
Keywords: Cc: mmitar@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: deSugar/should_compile/T4488
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

GHC should report unnecessary fromIntegral and other conversions. Sometimes you change the type later on and fromIntegral stays in the code but it is not necessary anymore. HLint cannot do that because it does not know about types.

Change History (7)

comment:1 Changed 9 years ago by simonpj

You'll need to be far more precise about what constitutes an "unnecessary" conversion, and what warning you expect to be produced!

comment:2 Changed 9 years ago by mitar

Unnecessary is for me when both types are equal. Unnecessary is for me when I can remove it and it still compiles. And warning should be just along the lines "unnecessary fromIntegral there and there". (And also for other similar conversion functions.)

comment:3 Changed 9 years ago by simonpj

Can you be precise? Which other conversion functions do you have in mind?

I don't think this would be too hard to implement.

comment:4 Changed 9 years ago by mitar

All defined in Prelude under Numeric Coercions and Overloaded Literals section:

fromInteger             :: (Num a) => Integer -> a
fromRational            :: (Fractional a) => Rational -> a
toInteger               :: (Integral a) => a -> Integer
toRational              :: (RealFrac a) => a -> Rational
fromIntegral            :: (Integral a, Num b) => a -> b
fromRealFrac            :: (RealFrac a, Fractional b) => a -> b

comment:5 Changed 9 years ago by igloo

Milestone: 7.2.1

comment:6 Changed 9 years ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: deSugar/should_compile/T4488

OK, your wishes are granted

Tue Nov 16 17:15:10 GMT 2010  simonpj@microsoft.com
  * Add warning for probable identities (fromIntegral and friends)
  
  See Trac #4488.  The basic idea is to check for
  
      fun :: ty -> ty
  
  where fun is one of
    toIntegerName     toRationalName
    fromIntegralName  realToFracName
  
  There's a (documented) flag to control it -fwarn-identities.
  Currently -Wall switches it on.

    M ./compiler/deSugar/Desugar.lhs -3 +7
    M ./compiler/deSugar/DsExpr.lhs -3 +37
    M ./compiler/main/DynFlags.hs -1 +4
    M ./compiler/prelude/PrelNames.lhs -7 +21
    M ./docs/users_guide/flags.xml +8
    M ./docs/users_guide/using.xml +15

I didn't do fromInteger and fromRational there are zillions of them generated by literals. ("1" means "fromInteger 1" etc.) And many of them are identities. I didn't think it was worth trying to distinguish user-written calls to fromInteger from these implicit ones, although that would be possible.

In compiling the libraries I found lots of redundant fromIntegral calls, plus one redundant toRational.

Simon

comment:7 Changed 9 years ago by mitar

Great! Thanks! I didn't know you are a genie of the lamp. ;-)

Note: See TracTickets for help on using tickets.