Opened 20 months ago

Closed 16 months ago

Last modified 14 months ago

#10751 closed task (fixed)

Implement Phase 1 of MonadFail Proposal (MFP)

Reported by: hvr Owned by: quchen
Priority: highest Milestone: 8.0.1
Component: Compiler Version: 7.10.2
Keywords: report-impact Cc: quchen, core-libraries-committee, ekmett, spl
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D1248
Wiki Page: prime:Libraries/Proposals/MonadFail

Description (last modified by simonpj)

See also

TODO At this point this ticket is mostly a reminder that stuff may need to be done for GHC 7.12

David Luposchainsky says that he plans to work on it.

Change History (25)

comment:1 Changed 19 months ago by simonpj

Description: modified (diff)

comment:2 Changed 19 months ago by simonpj

Description: modified (diff)

comment:3 Changed 19 months ago by quchen

Current status:

  • Warnings are issued for missing MonadFail instances. These warnings are on by default and can be toggled with a flag.
  • The (->)r Monad doesn't yield the correct error message yet: it displays as "m r" in the "missing MonadFail" message.
  • There is a flag to turn on the actual desugaring. It's turned off by default, but can be used to get a hard error if something's been forgotten.
  • The testsuite has been mostly fixed (not sure whether the failing tests are our fault, didn't check it yet)
  • We have lots of WIP commits that aren't very good versioning practice, so the actual patch will need to be squashed down to a handful of useful ones.

comment:4 Changed 19 months ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:5 Changed 19 months ago by quchen

Draft submitted to Phab: https://phabricator.haskell.org/D1248

comment:6 Changed 19 months ago by hvr

Owner: set to quchen

comment:7 Changed 19 months ago by hvr

Status: newpatch

comment:8 Changed 19 months ago by hvr

Differential Rev(s): Phab:D1248
Priority: normalhighest

comment:9 Changed 19 months ago by goldfire

Sorry for coming to the party late, but is there an up-to-date proposal I could read here? The link in the original post goes to a reddit page, which has links to an "original propsal" and an "update 1", both of which seem to have open questions, and neither of which are editable.

I don't have any objections -- just trying to stay informed. Thanks!

comment:10 Changed 19 months ago by quchen

Discussion of the initial proposal went pretty dead soon, and there was close to no response for the update, so we left it at that (considering the few suggestions we got and that people actually cared about) and fleshed it out on IRC. We modified the transitional strategy based on what we had a bit, and it's in https://github.com/quchen/articles/blob/master/monad_fail.md#transitional-strategy, which is what I'm implementing right now.

Objections like the purpose of failure - such exception vs. error state - are orthogonal to the proposal, whose main goal by a huge margin is getting fail out of Monad. We can revisit such issues when MonadFail is established, e.g. by adding a new function to the MonadFail class that is called by the compiler, and one that's supposed to be used by the users explicitly.

comment:11 Changed 19 months ago by simonpj

Cc: core-libraries-committee ekmett added

Just to be clear, this is a Core Libraries Committee matter, so whatever the proposal is, it should be signed off by them.

Simon

comment:12 Changed 18 months ago by ekmett

My sense is that support within the committee or this proposal is very high, based on various discussions.

I will however, take a moment to formally poll for objections within the committee just to make absolutely sure before I commit to signing off.

That said, would it be possible to move the proposal from a github page to the Haskell Wiki for posterity?

That would make it easier to express questions about minor points, e.g. whether ViewPatterns/PatternSynonyms should always be considered failable or if they should only be considered failable if their right hand side is.

e.g.

pattern Foo = ((),())

is clearly an unfailable pattern.

as is

foo (snd -> x) = ...

as 'x' is unfailable.

These compose for useful patterns like

pattern Polar x y <- (magnitude &&& phase -> (x,y)) where
  Polar x y = mkPolar x y

Those items there are probably the primary sticking point I have personally, but in broad strokes I personally absolutely adore this proposal.

comment:13 Changed 18 months ago by quchen

would it be possible to move the proposal from a github page to the Haskell Wiki for posterity?

Of course, I should have done that earlier. I'll get to it within a few hours.

comment:15 Changed 18 months ago by spl

Cc: spl added

comment:16 Changed 18 months ago by spl

I worked a bit on the wiki proposal.

I think it would be good to indicate (1) which MonadFail instances will be created and (2) which types with Monad instances will not have MonadFail instances. Since this change affects a number of important packages, those packages should probably be included.

Also, I'm not clear on the conclusion for having Monad vs. Applicative as the superclass constraint on MonadFail. I tried to develop the discussion text on this a bit. Since we're getting ApplicativeDo soon, I think it would be useful to work out whether and how these two features are related. I didn't see anything on ApplicativeDo referring to the desugaring of pattern matching.

comment:17 Changed 17 months ago by Herbert Valerio Riedel <hvr@…>

In 8c80dcc/ghc:

base: Add new Control.Monad.Fail module (re #10751)

This is based on David's initial patch augmented by more extensive
Haddock comments.

This has been broken out of D1248 to reduce its size
by splitting the patch into smaller logical pieces.

On its own, this new module does nothing interesting yet.
Later patches will add support for a different desugaring of
`do`-blocks, at which point the new `MonadFail` class will
become more useful.

Reviewed By: ekmett, austin

Differential Revision: https://phabricator.haskell.org/D1424

comment:18 Changed 16 months ago by Ben Gamari <bgamari.foss@…>

In 233d131/ghc:

MonadFail proposal, phase 1

This implements phase 1 of the MonadFail proposal (MFP, #10751).

- MonadFail warnings are all issued as desired, tunable with two new flags
- GHC was *not* made warning-free with `-fwarn-missing-monadfail-warnings`
  (but it's disabled by default right now)

Credits/thanks to
- Franz Thoma, whose help was crucial to implementing this
- My employer TNG Technology Consulting GmbH for partially funding us
  for this work

Reviewers: goldfire, austin, #core_libraries_committee, hvr, bgamari, fmthoma

Reviewed By: hvr, bgamari, fmthoma

Subscribers: thomie

Projects: #ghc

Differential Revision: https://phabricator.haskell.org/D1248

GHC Trac Issues: #10751

comment:19 Changed 16 months ago by bgamari

Resolution: fixed
Status: patchclosed

comment:20 Changed 16 months ago by hvr

Wiki Page: prime:Libraries/Proposals/MonadFail

comment:21 Changed 16 months ago by Ben Gamari <ben@…>

In c7a058fb/ghc:

User's Guide: Add links to MFP wiki page

Test Plan: IIAM

Reviewers: austin, bgamari, quchen

Reviewed By: bgamari, quchen

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D1529

GHC Trac Issues: #10751

comment:22 Changed 16 months ago by Herbert Valerio Riedel <hvr@…>

In d25f8535/ghc:

Update transformers submodule

Most notably this pulls in `MonadFail` instances
(see also #10751)

- add MonadFail instance for ContT
- re-order methods for consistency
- Add `MonadFail` instances
- Canonicalise Monad instances
- instance Bifunctor Constant

comment:23 Changed 14 months ago by Herbert Valerio Riedel <hvr@…>

In fd6dd41c/ghc:

Implement `-Wnoncanonical-monadfail-instances` warning

The MonadFail proposal implemented so far via #10751 only warns about
missing `MonadFail` instances based on existence of failible pattern
matches in `do`-blocks.

However, based on the noncanonical Monad warnings implemented via #11150
we can provide a different mechanism for detecting missing `MonadFail`
instances quite cheaply. That is, by checking for canonical `fail` definitions.

In the case of `Monad`/`MonadFail`, we define the canonical implementation of
`fail` to be such that the soft-deprecated method shall (iff overridden) be
defined in terms of the non-deprecated method. Consequently, in case of
`MonadFail`, the `Monad(fail)` method shall be defined as alias of
the `MonadFail(fail)` method.

This allows us at some distant point in the future to remove `fail` from
the `Monad` class, while having GHC ignore/tolerate such literal canonical
method definitions.

Reviewed By: bgamari, RyanGlScott

Differential Revision: https://phabricator.haskell.org/D1838

comment:24 Changed 14 months ago by Ben Gamari <ben@…>

In 132c2089/ghc:

Rename -Wmissing-monadfail-instance to plural-form

This warning flag was recently introduced as part of #10751. However,
it was missed during code-review that almost all existing warning
flags use a plural-form, so for consistency this commit renames
that warning flag to `-Wmissing-monadfail-instances`.

Test Plan: local validate (still running)

Reviewers: quchen, goldfire, austin, bgamari

Reviewed By: bgamari

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D1842

GHC Trac Issues: #10751

comment:25 Changed 14 months ago by bgamari

Renaming merged to ghc-8.0.

Note: See TracTickets for help on using tickets.