[GHC] #11128: New `-fwarn-noncanonical-monad-instances` warning
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature | Status: new request | Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- placeholder -- I needed a ticket number; will flesh out description asap -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature request | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1516 Wiki Page: | -------------------------------------+------------------------------------- Changes (by hvr): * status: new => patch * differential: => Phab:D1516 Old description:
placeholder -- I needed a ticket number; will flesh out description asap
New description: (see description in Phab:D1516 for the meantime) -- -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128#comment:1> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature request | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1516 Wiki Page: | -------------------------------------+------------------------------------- Comment (by nomeata): Do you intentionally disallow `return x = pure x`, which some people might write? (No strong opinion here, having precisely one official way is probably desirable, but let’s at least document that it has been given thought.) -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128#comment:2> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature request | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1516 Wiki Page: | -------------------------------------+------------------------------------- Comment (by hvr): Replying to [comment:2 nomeata]:
Do you intentionally disallow `return x = pure x`, which some people might write? (No strong opinion here, having precisely one official way is probably desirable, but let’s at least document that it has been given thought.)
Yes, that was quite intentional, and there are a couple of reasons actually: - I found only *one* single `return a = pure a` on Hackage when I grepped for `rgrep 'return *[a-z] *= *pure *[a-z]'`; this is maybe because any instructions I saw for making code AMP-proof used rather pointsfree equational style e.g. (`return = pure`), and the Haddock documentation in `base` uses that style too - the warning is easier to document, implement, and less corner-cases to worry about - "canonical" somewhat implies we want a single unique form here, this also has the benefit of being able to have a simpler regexp to find all `return *= *pure` patterns in your codebase (e.g. if you want to drop those when base<4.8 compat doesn't matter anymore) -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128#comment:3> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature request | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1516 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Herbert Valerio Riedel <hvr@…>): In [changeset:"f09f2470a76bb08b7f51d2f5663daa672b86f618/ghc" f09f2470/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="f09f2470a76bb08b7f51d2f5663daa672b86f618" Implement new `-fwarn-noncanonical-monad-instances` Warn about incoherent/non-canonical 'Applicative'/'Monad' instance declarations. Specifically the following invariants are checked: In 'Monad' instances declarations warn if the any of the following conditions does not hold: * If `return` is overridden it must be canonical (i.e. `return = pure`). * If `(>>)` is overridden it must be canonical (i.e. `(>>) = (*>)`). In 'Applicative' instance declarations: * Warn if 'pure' is defined backwards (i.e. `pure = return`). * Warn if '(*>)' is defined backwards (i.e. `(*>) = (>>)`). NB, this warning flag is not enabled via `-Wall` nor `-Wcompat`. This addresses #11128 Reviewers: quchen, austin, bgamari Reviewed By: bgamari Differential Revision: https://phabricator.haskell.org/D1516 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128#comment:4> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature request | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1516 Wiki Page: | -------------------------------------+------------------------------------- Description changed by hvr: Old description:
(see description in Phab:D1516 for the meantime)
New description: When declaring `Applicative` and `Monad` instances, there's a degree of freedom in which way to define `return`,`pure`,`(>>)`,`(*>)`. For instance, defining {{{#!hs instance Applicative T1 where pure = return (<*>) = ap instance Monad T1 where return = ... (>>=) = ... (>>) = ... }}} is ok, but it's leaves `(*>)` with a possibly less optimised version than `(>>)`. This can cause performance regressions when generalising code from `Monad` to `Applicative`. Moreover, starting with `base-4.8`, the `return` method gained a default implementation `return = pure` which follows the preferred or "canonical" direction of having implementations flow from superclasses to their subclasses. So this warning is a "lint"-style check to help detect `Monad` instances where the definitions of `return`/`(>>)` are not canonical, i.e. don't match `return = pure` and `(>>) = (*>)` respectively. -- -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128#comment:5> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature request | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1516 Wiki Page: | -------------------------------------+------------------------------------- Description changed by hvr: Old description:
When declaring `Applicative` and `Monad` instances, there's a degree of freedom in which way to define `return`,`pure`,`(>>)`,`(*>)`. For instance, defining
{{{#!hs instance Applicative T1 where pure = return (<*>) = ap
instance Monad T1 where return = ... (>>=) = ... (>>) = ... }}}
is ok, but it's leaves `(*>)` with a possibly less optimised version than `(>>)`. This can cause performance regressions when generalising code from `Monad` to `Applicative`.
Moreover, starting with `base-4.8`, the `return` method gained a default implementation `return = pure` which follows the preferred or "canonical" direction of having implementations flow from superclasses to their subclasses.
So this warning is a "lint"-style check to help detect `Monad` instances where the definitions of `return`/`(>>)` are not canonical, i.e. don't match `return = pure` and `(>>) = (*>)` respectively.
New description: When declaring `Applicative` and `Monad` instances, there's a degree of freedom in which way to define `return`,`pure`,`(>>)`,`(*>)`. For instance, defining {{{#!hs instance Applicative T1 where pure = return (<*>) = ap instance Monad T1 where return = ... (>>=) = ... (>>) = {- optimised impl -} }}} is ok, but this leaves `(*>)` with a possibly less optimised version than `(>>)`. This can cause performance regressions when generalising code from `Monad` to `Applicative`. Moreover, starting with `base-4.8`, the `return` method gained a default implementation `return = pure` which follows the preferred or "canonical" direction of having implementations flow from superclasses to their subclasses. A proper "canonical" definition of `T1` is consequently: {{{#!hs instance Applicative T1 where pure = ... (<*>) = ap (*>) = {- optimised impl -} instance Monad T1 where return = pure -- can be left off since base-4.8 (>>=) = ... (>>) = (*>) -- NB: default impl of (>>) /= (*>) }}} So this warning is a "lint"-style check to help detect `Monad` instances where the definitions of `return`/`(>>)` are not canonical, i.e. don't match `return = pure` and `(>>) = (*>)` respectively. -- -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128#comment:6> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature request | Status: closed Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1516 Wiki Page: | -------------------------------------+------------------------------------- Changes (by hvr): * status: patch => closed * resolution: => fixed -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128#comment:7> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#11128: New `-fwarn-noncanonical-monad-instances` warning -------------------------------------+------------------------------------- Reporter: hvr | Owner: hvr Type: feature request | Status: closed Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1516 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Herbert Valerio Riedel <hvr@…>): In [changeset:"986ceb1679b501414b996c520b08ce929a40f94c/ghc" 986ceb16/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="986ceb1679b501414b996c520b08ce929a40f94c" Implement new `-fwarn-noncanonical-monoid-instances` This is similiar to the `-fwarn-noncanonical-monad-instances` warning implemented via #11128, but applies to `Semigroup`/`Monoid` instead and the `(<>)`/`mappend` methods (of which `mappend` is planned to move out of `Monoid` at some point in the future being redundant and thus error-prone). This warning is contained in `-Wcompat` but not in `-Wall`. This addresses #11150 Reviewed By: quchen Differential Revision: https://phabricator.haskell.org/D1553 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11128#comment:8> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
participants (1)
-
GHC