Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#10190 closed feature request (fixed)

Add a Monad instance for ((,) w)

Reported by: fumieval Owned by: fumieval
Priority: normal Milestone: 8.0.1
Component: libraries/base Version:
Keywords: report-impact Cc: ekmett, hvr, core-libraries-committee@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Now Monoid is defined in GHC.Base; We can define a reasonable Monad instance for (,) w, as a writer monad.

Attachments (2)

0001-Add-missing-Monad-instance-for-a.patch (1.4 KB) - added by fumieval 3 years ago.
0001-Add-missing-Monad-instance-for-a.2.patch (1.4 KB) - added by fumieval 3 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 3 years ago by fumieval

Status: newpatch

comment:2 Changed 3 years ago by fumieval

Cc: core-libraries-committee@… added
Component: libraries/baseCore Libraries

comment:3 Changed 3 years ago by ekmett

No objection from me.

This has passed the libraries@ proposal process on at least 2 separate occasions in recent memory, but had been blocked by the fact that Monoid wasn't in scope where the Monad class was defined.

We have the compatible Applicative instance.

I had honestly thought we already had the instance in 7.10.

comment:4 Changed 3 years ago by snoyberg

I have no objection to this change.

comment:5 in reply to:  1 Changed 3 years ago by hvr

Component: Core Librarieslibraries/base
Milestone: 7.12.1
Owner: set to fumieval
Type: bugfeature request
Version: 7.8.4

Replying to fumieval:

Re your patch, the implementation LGTM, but the meta-information needs improvement:

  • missing a libraries/base/changelog.md entry
  • commit message needs to reference #10190
  • commit message should contain link(s) to the threads which proposed this addition (and passed)

Please also consider using Phabricator, as then we also get validation via the build-bot that your patch doesn't need additional tweaks somewhere (e.g. in the testsuite)

Changed 3 years ago by fumieval

Changed 3 years ago by fumieval

comment:6 Changed 3 years ago by fumieval

Sorry, I couldn't find the thread which proposed this instance.

comment:7 Changed 3 years ago by ekmett

Here are two of the proposals that went through the libraries@ process over the years on the topic:

My google-fu is failing me when it comes to finding the rest quickly.

comment:8 Changed 3 years ago by hvr

I'll take it from here, thanks

comment:9 Changed 3 years ago by Herbert Valerio Riedel <hvr@…>

In 9db005a444722e31aca1956b058e069bcf3cacbd/ghc:

Add Monad instance for `((,) a)` (#10190)

This was proposed a couple of times in the past, e.g.

 - https://mail.haskell.org/pipermail/libraries/2011-November/017153.html
 - https://mail.haskell.org/pipermail/libraries/2013-July/020446.html

but its implementation had been blocked by the fact that `Monoid` wasn't
in scope where the `Monad` class was defined. Since the AMP/FTP restructuring
this is no longer the case.

comment:10 Changed 3 years ago by hvr

Resolution: fixed
Status: patchclosed

comment:11 Changed 3 years ago by hvr

Keywords: report-impact added

comment:12 Changed 3 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

Note: See TracTickets for help on using tickets.