[GHC] #9939: Warn for duplicate superclass constraints
#9939: Warn for duplicate superclass constraints -------------------------------------+------------------------------------- Reporter: crockeea | Owner: Type: feature | Status: new request | Milestone: Priority: normal | Version: 7.8.3 Component: Compiler | Operating System: Unknown/Multiple Keywords: | Type of failure: None/Unknown Architecture: | Blocked By: Unknown/Multiple | Related Tickets: Test Case: | Blocking: | Differential Revisions: | -------------------------------------+------------------------------------- With the following code, GHC warns that there are duplicate constraints: {{{#!hs {-# LANGUAGE FlexibleInstances, UndecidableInstances #-} module Foo where class Foo a where foo :: Int -> a instance (Integral a, Integral a) => Foo a where foo x = (fromIntegral x) `div` 2 }}} However, when writing the code, I might start with: {{{#!hs {-# LANGUAGE FlexibleInstances, UndecidableInstances #-} module Foo where class Foo a where foo :: Int -> a instance Foo a where foo x = (fromIntegral x) `div` 2 }}} without any constraints on the instance. GHC complains that it needs `Num a` and `Integral a`, but of course `Num` is implied by `Integral`. I'm ''not'' asking that GHC figure this out on its own and only request the strongest constraint necessary. Rather, I'm suggesting that ''if'' I followed GHC's suggestion and wrote {{{#!hs {-# LANGUAGE FlexibleInstances, UndecidableInstances #-} module Foo where class Foo a where foo :: Int -> a instance (Num a, Integral a) => Foo a where foo x = (fromIntegral x) `div` 2 }}} then GHC should warn {{{ Duplicate constraint(s): Num a In the context: (Num a, Integral a) (Num a) is implied by (Integral a) }}} or something similar. The motivation for this feature request is that in large instances/programs, it is difficult for a human to keep track of superclasses. In large instances, GHC tends to request "weak" constraints first (say `Num`), then ask for progressively stronger constraints (say `Integral`). Again, I'm not suggesting that behavior should change. However, it tends to lead to instances that look like `(Num a, Real a, RealFrac a, RealFloat a) => ...` if by chance I happened to use methods from each class. It seems fairly simple for GHC to look at each constraint for an instance (or function), trace back up the class hierarchy to get a set of all implied constraints, and then warn if one set is a subset of another. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/9939> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#9939: Warn for duplicate superclass constraints -------------------------------------+------------------------------------- Reporter: crockeea | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Blocked By: | Test Case: Related Tickets: | Blocking: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by goldfire): Another example I hit recently. `Language.Haskell.TH.Syntax` used to have {{{ class (Applicative m, Monad m) => Quasi m where ... }}} When I saw this recently, I realized the `Applicative m` constraint is now redundant and so I removed it. However, it would have been helpful if GHC had issued a warning. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/9939#comment:1> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#9939: Warn for duplicate superclass constraints -------------------------------------+------------------------------------- Reporter: crockeea | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Blocked By: | Test Case: Related Tickets: | Blocking: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by Simon Peyton Jones <simonpj@…>): In [changeset:"32973bf3c2f6fe00e01b44a63ac1904080466938/ghc"]: {{{ #!CommitTicketReference repository="ghc" revision="32973bf3c2f6fe00e01b44a63ac1904080466938" Major patch to add -fwarn-redundant-constraints The idea was promted by Trac #9939, but it was Christmas, so I did some recreational programming that went much further. The idea is to warn when a constraint in a user-supplied context is redundant. Everything is described in detail in Note [Tracking redundant constraints] in TcSimplify. Main changes: * The new ic_status field in an implication, of type ImplicStatus. It replaces ic_insol, and includes information about redundant constraints. * New function TcSimplify.setImplicationStatus sets the ic_status. * TcSigInfo has sig_report_redundant field to say whenther a redundant constraint should be reported; and similarly the FunSigCtxt constructor of UserTypeCtxt * EvBinds has a field eb_is_given, to record whether it is a given or wanted binding. Some consequential chagnes to creating an evidence binding (so that we record whether it is given or wanted). * AbsBinds field abs_ev_binds is now a *list* of TcEvBiinds; see Note [Typechecking plan for instance declarations] in TcInstDcls * Some significant changes to the type checking of instance declarations; Note [Typechecking plan for instance declarations] in TcInstDcls. * I found that TcErrors.relevantBindings was failing to zonk the origin of the constraint it was looking at, and hence failing to find some relevant bindings. Easy to fix, and orthogonal to everything else, but hard to disentangle. Some minor refactorig: * TcMType.newSimpleWanteds moves to Inst, renamed as newWanteds * TcClassDcl and TcInstDcls now have their own code for typechecking a method body, rather than sharing a single function. The shared function (ws TcClassDcl.tcInstanceMethodBody) didn't have much code and the differences were growing confusing. * Add new function TcRnMonad.pushLevelAndCaptureConstraints, and use it * Add new function Bag.catBagMaybes, and use it in TcSimplify }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/9939#comment:2> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#9939: Warn for duplicate superclass constraints -------------------------------------+------------------------------------- Reporter: crockeea | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Blocked By: | Test Case: Related Tickets: | Blocking: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by Simon Peyton Jones <simonpj@…>): In [changeset:"c790fe87be648f420c63099934852013a4e8a8f7/ghc"]: {{{ #!CommitTicketReference repository="ghc" revision="c790fe87be648f420c63099934852013a4e8a8f7" Test Trac #9939 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/9939#comment:3> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#9939: Warn for duplicate superclass constraints -------------------------------------+------------------------------------- Reporter: crockeea | Owner: Type: feature request | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.8.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Blocked By: | Test Case: Related Tickets: | typecheck/should_compile/T9939 | Blocking: | Differential Revisions: -------------------------------------+------------------------------------- Changes (by simonpj): * status: new => closed * testcase: => typecheck/should_compile/T9939 * resolution: => fixed Comment: Well, I went ''way'' overboard on this, but it was fun! Thanks for the suggestion. It showed up masses of unnecessary constraints in the compiler itself, and in many libraries, as you can see from {{{ commit c409b6f30373535b6eed92e55d4695688d32be9e Author: Simon Peyton Jones <simonpj@microsoft.com> Date: Tue Jan 6 13:46:35 2015 +0000 Remove redundant constraints from libraries, discovered by -fwarn- redundant-constraints libraries/array | 2 +- libraries/base/Data/Data.hs | 8 +++---- libraries/base/Data/Foldable.hs | 4 ++-- libraries/base/GHC/Arr.hs | 46 ++++++++++++++++++------------------- libraries/base/GHC/Event/Array.hs | 4 ++-- libraries/base/GHC/IOArray.hs | 4 ++-- libraries/base/GHC/Real.hs | 6 ++--- libraries/base/Text/Printf.hs | 2 +- libraries/deepseq | 2 +- libraries/hoopl | 2 +- libraries/parallel | 2 +- mk/validate-settings.mk | 3 +++ }}} and {{{ commit 39337a6d97c853a88fa61d6b12a04eb8c2e5984f Author: Simon Peyton Jones <simonpj@microsoft.com> Date: Mon Jan 5 16:57:01 2015 +0000 Remove redundant constraints in the compiler itself, found by -fwarn- redundant-constraints compiler/basicTypes/Name.hs | 3 ++- compiler/cmm/CmmExpr.hs | 8 ++++---- compiler/cmm/Hoopl/Dataflow.hs | 2 +- compiler/coreSyn/TrieMap.hs | 4 ++-- compiler/deSugar/MatchLit.hs | 2 +- compiler/ghci/ByteCodeItbls.hs | 6 ++++-- compiler/ghci/Linker.hs | 2 +- compiler/hsSyn/HsDecls.hs | 8 +++----- compiler/hsSyn/HsExpr.hs | 8 ++++---- compiler/main/CmdLineParser.hs | 2 +- compiler/main/GHC.hs | 10 +++++++--- compiler/main/GhcMonad.hs | 17 ++++++++++++---- compiler/main/InteractiveEval.hs | 3 +-- compiler/nativeGen/RegAlloc/Graph/SpillClean.hs | 11 ++++------- compiler/nativeGen/RegAlloc/Linear/Main.hs | 4 ++-- compiler/nativeGen/SPARC/Base.hs | 2 +- compiler/typecheck/TcRnMonad.hs | 3 ++- compiler/types/CoAxiom.hs | 2 +- compiler/utils/Binary.hs | 2 +- compiler/utils/GraphColor.hs | 6 +++--- compiler/utils/GraphOps.hs | 24 ++++++++++------------- compiler/utils/GraphPpr.hs | 9 ++++----- compiler/utils/Maybes.hs | 4 ++++ compiler/utils/Serialized.hs | 4 ++-- compiler/utils/UniqSet.hs | 2 +- }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/9939#comment:4> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
participants (1)
-
GHC