[GHC] #8132: Warning for Typeable instances misplaced
#8132: Warning for Typeable instances misplaced -------------------------+------------------------------------------------- Reporter: | Owner: scottgw | Status: new Type: bug | Milestone: Priority: | Version: 7.7 normal | Operating System: Unknown/Multiple Component: | Type of failure: Incorrect warning at Compiler | compile-time Keywords: | Test Case: Architecture: | Blocking: Unknown/Multiple | Difficulty: | Unknown | Blocked By: | Related Tickets: | -------------------------+------------------------------------------------- There is currently a warning for hand-rolled instances of Typeable. However, this leads to some pretty surprising error messages: because the instance is ignored any code that relies on that instance in the same module just fails, which appears pretty weird to the user. {{{ #!haskell import Data.Typeable data K = K instance Typeable K where typeRep _ = undefined ex :: TypeRep ex = typeRep (undefined :: Proxy K) }}} Gives as an error: {{{ No instance for (Typeable * K) arising from a use of ‛typeRep’ In the expression: typeRep (undefined :: Proxy K) In an equation for ‛ex’: ex = typeRep (undefined :: Proxy K) }}} Which is of course surprising because the user just defined the instance. After commenting out the code that uses the instance, one gets: {{{ TypEx.hs:1:1: Warning: Typeable instances can only be derived; ignoring the following instance: instance Typeable * K -- Defined at TypEx.hs:5:10 }}} This information should clearly be somehow presented before or with the previous error if the user is to have any way to figure out what is going on. I suppose this could be fixed by: 1. Turning the Typeable instance warning into an error 2. Checking the missing instance in the existing error for Typeable and suggest that the user should look for hand written instances as a source of the problem. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Changes (by dreixel): * owner: => dreixel Comment: Thanks for the report. I think I'd fix it by adapting the error to include a notice stating that there may be handwritten instances being ignored. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by scottgw): The disadvantage of amending the existing error is that it separates cause of the error from its from it symptom, and also doesn't pinpoint the cause reliably (You didn't write an instance! Or maybe you did and I ignored it!). The disadvantage of making a new error for this case is that clients who don't need the instance won't be able to compile until the package is fixed. I'm not sure what should constitute an error, but if GHC is just throwing these instances away because they are not allowed, then perhaps it makes sense to just disallow them. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by simonpj): I think we should turn the warning into an error. We really, really don't want hand-written `Typeable` instances. If there are any, we should just report an error. Making it into an error rather than a warning will ensure that it is displayed even if there are other errors. (Indeed, if there are errors in the instances, we may not even proceed to typecheck the value declarations at all... I'm not sure.) Pedro, would you be up for making that (easy) change? Thanks. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by dreixel): Replying to [comment:3 simonpj]:
I think we should turn the warning into an error.
But that breaks backwards compatibility in a serious way. Lots of code lying around gives manual `Typeable` instances right now, especially because that's the only way to define such instances in some cases. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by simonpj): Oh bother. We want to give a pretty strong incentive to change those `Typeable` instances. At least the warning should say "This warning will become an error in GHC 7.10; please use "deriving Typeable" instead". Then let's re-mile-stone this ticket for 7.10 with highest priority. Until then maybe it's the lesser evil to have the problem described by this ticket. (An alternative would be to have warnings that are displayed willy-nilly, but there's no easy way to do that.) Might you do those things? Thanks Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by scottgw): Maybe I am misunderstanding but it seems like backwards compatibility is already broken, twice: 1. Backwards compatibility is already broken by changing `typeOf` to `typeRep` in Typeable. So anyone coming from 7.6 is already going to have a compile error on any manually defined instance. 2. Ignoring that, backwards compatibility is already broken by ignoring manually defined instances. Making this an error just turns a 3 step fixing process (typeOf -> typeRep, discover complaints about missing instance scratch head, use deriving Typeable) into a 2 step one (typeOf -> typeRep, use deriving Typeable). The other solution is to not have it ignore the hand written instances, which would also bring it back down to even 1 step (typeOf -> typeRep). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by simonpj): Yurgh. You are quite right. It's really hard to move on ''and'' maintain backwards compatibility! I'm open to advice. Two alternatives make sense to me: * Maintain total backward compat for a single release, by putting the new stuff in `Data.NewTypeable` for one release (with heavy deprecation messages), and ''then'' in 7.10 moving it to `Data.Typeable`, while making the old `Data.Typeable` into `Data.OldTypeable`. * Just make the breaking change now (which as you point out, we have already done in HEAD). In that case we should make manual `Typeable` instances an error right away. I'd prefer the latter. In practice each release of GHC does typically require some changes wrt base, and I doubt that 7.8 will be an exception to that. But I'm sensitive to what users want. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by scottgw): Well, assuming hand-coding {{{Typeable}}} instances is just as dangerous as it was last release, then the first option is more appropriate. It will allow more people to quickly move to 7.8 and find all the nasty bugs/great features, which is what I guess everyone wants. In this case, I would also keep {{{NewTypeable}}} in 7.10 as well, just re-exporting {{{Typeable}}}. Then packages don't have a hard partition between revisions and it gives maintainers a release to fix the 'typo'. Otherwise it makes the advice in 7.8 strange: the old {{{Data.Typeable}}} is being deprecated, please use the (deprecated) {{{Data.NewTypeable}}}. The churn will be constant in either case, but this keeps the user-facing breakages down. I would be happy to help make the changes as well, if someone were to point me to the relevant places. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by dreixel): Replying to [comment:7 simonpj]:
I'm open to advice. Two alternatives make sense to me:
Well, I still think keeping the current behaviour in HEAD (perhaps with an improved warning in this case) is an option. Handwritten `Typeable` instances will indeed break, but most handwritten instances I've seen are for `Typeable1` etc. We could also forego the `typeOf -> typeRep` name change; I don't think that it's really necessary, and it might reduce the amount of code that breaks. Or we can let `Typeable` have both (the new) `typeRep` and (the old) `typeOf`, with the latter having a default definition in terms of the former. This being said, if I would have to choose between Simon's two options, I'd also prefer making the breaking change right now. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by simonpj): It's all coming back; sorry to be so slow. It's true that 95% of all users will see no change because they already say `deriving( Typeable )`. There are very few handwritten instances. It's true that changing the name of the method from `typeOf` to `typeRep` is an unforced change. To me `typeRep` makes more sense; `(typeOf p)` doesn't actually return the type of `p`, which is `Proxy t` for some t. But we could easily include a (deprecated) function {{{ typeOf :: Typeable a => Proxy a -> TypeRep typeOf = typeRep {-# DEPRECATED typeOf "Use typeRep instead" #-} }}} Together that would mean that only a tiny fraction of uses would break; they can if necessary recover the old behaviour via `OldTypeable`; and users would have a release cycle to update to `typeRep`. Sound fair? It's more or less what we have in HEAD now, apart from making manual `Typeable` instances into errors. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by scottgw): That sounds like a reasonable approach: I checked with a (probably insufficient) regex on the latest Hackage archive and there are indeed not many (~90) packages that appear to write an instance by hand for {{{Typeable}}}. Out of curiosity is there guidance on when/how to introduce breaking changes in GHC? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: highest | Status: new Component: Compiler | Milestone: 7.8.1 Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Changes (by simonpj): * priority: normal => highest * milestone: => 7.8.1 Comment: OK I think we are agreed on * Hand-rolled instances of `Typeable` become an error. * Deprecated, top-level definition of `typeOf` is provided. Pedro, could you do that? We'll need it in the next week or two for 7.8. Thanks Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: highest | Status: new Component: Compiler | Milestone: 7.8.1 Resolution: | Version: 7.7 Operating System: Unknown/Multiple | Keywords: Type of failure: Incorrect warning at | Architecture: compile-time | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Comment (by dreixel): Yes, I'll do it. But I think we agreed before that there's no need to deprecate `typeOf` and its friends, as they are nice shortcuts for `typeRep` anyway. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced
-------------------------------------------------+-------------------------
Reporter: scottgw | Owner:
Type: bug | dreixel
Priority: highest | Status: new
Component: Compiler | Milestone: 7.8.1
Resolution: | Version: 7.7
Operating System: Unknown/Multiple | Keywords:
Type of failure: Incorrect warning at | Architecture:
compile-time | Unknown/Multiple
Test Case: | Difficulty:
Blocking: | Unknown
| Blocked By:
| Related Tickets:
-------------------------------------------------+-------------------------
Comment (by Jose Pedro Magalhaes
#8132: Warning for Typeable instances misplaced
-------------------------------------------------+-------------------------
Reporter: scottgw | Owner:
Type: bug | dreixel
Priority: highest | Status: new
Component: Compiler | Milestone: 7.8.1
Resolution: | Version: 7.7
Operating System: Unknown/Multiple | Keywords:
Type of failure: Incorrect warning at | Architecture:
compile-time | Unknown/Multiple
Test Case: | Difficulty:
Blocking: | Unknown
| Blocked By:
| Related Tickets:
-------------------------------------------------+-------------------------
Comment (by Jose Pedro Magalhaes
#8132: Warning for Typeable instances misplaced -------------------------------------------------+------------------------- Reporter: scottgw | Owner: Type: bug | dreixel Priority: highest | Status: Component: Compiler | closed Resolution: fixed | Milestone: 7.8.1 Operating System: Unknown/Multiple | Version: 7.7 Type of failure: Incorrect warning at | Keywords: compile-time | Architecture: Test Case: | Unknown/Multiple Blocking: | Difficulty: | Unknown | Blocked By: | Related Tickets: -------------------------------------------------+------------------------- Changes (by dreixel): * status: new => closed * resolution: => fixed Comment: Done. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8132: Warning for Typeable instances misplaced -------------------------------------+------------------------------------- Reporter: scottgw | Owner: dreixel Type: bug | Status: closed Priority: highest | Milestone: 7.8.1 Component: Compiler | Version: 7.7 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect | Unknown/Multiple warning at compile-time | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari): In the new type-indexed `Typeable` scheme (see #11011) we will hide `typeOf#` altogether. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8132#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC