TypeChecking of Monad patterns incorrect with RebindableSyntax and OverloadedStrings

Using GHC 8.4.3, I'd expect the following to work:

{-# LANGUAGE RebindableSyntax, OverloadedStrings #-}

module Fail where

import Prelude hiding (fail)

foo x = do
    Just y <- x
    return y

newtype Text = Text String

fail :: Text -> a
fail (Text x) = error x

fromString :: String -> Text
fromString = Text

But it fails with:

Fail.hs:8:5-15: error:
    * Couldn't match expected type `[Char]' with actual type `Text'
    * In a stmt of a 'do' block: Just y <- x
      In the expression:
        do Just y <- x
           return y
      In an equation for `foo':
          foo x
            = do Just y <- x
                 return y
  |
8 |     Just y <- x
  |     ^^^^^^^^^^^

Given the enabled extensions, I'd expect foo to desugar as:

foo x = x >>= \v -> case v of
    Just y -> return y
    _ -> fail (fromString "pattern match error")

But looking at TcMatches.tcMonadFailOp it checks the fail operation (which is literally fail) takes an argument of type tyString (e.g. [Char]). One way around that would be to make the "fail-op" being passed around be fail . fromString if the appropriate extensions are enabled.

Trac metadata

Trac field	Value
Version	8.4.3
Type	Bug
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Compiler
Test case
Differential revisions
BlockedBy
Related
Blocking
CC	ndmitchell@gmail.com
Operating system
Architecture

changed milestone to %8.6.1

changed weight to 5

added Tbug Trac import labels

Could be. But would it not be simpler for you to provide a fail with type String -> m a? It's not hard to do! I'm not seeing a strong motivation for doing this in the compiler.

The use case is for Prelude replacement modules that seek to switch the type of [Char] to something like Text. A standard way is to define your own Monad/MonadFail class which has fail :: Text -> m a in it. If that class is forced to have fail :: [Char] -> m a instead (as it is now) then all your users have to implement a function working on [Char], even though for everything else in your custom library, they never see [Char] and your custom Prelude has no other [Char] related functions.

In the particular example I'm working on, the Char type has been eliminated entirely, aside from the fail function.

But all you have to do is define fail = myFail . fromString and you are done.

I suppose you are going to say that you don't want two variants of fail in scope. That is a bit more convincing. But now you may get new mysterious messages about missing IsString instances arising from invisible code.

Make a GHC proposal!

Indeed, you can't use the name fail, which is unfortunate. The mysterious messages about things already happens - I get an error about [Char] vs Text - so the IsString thing is just the same consequence.

I'd view this as a bug fix (overloaded strings should imply even generated strings are overloaded, if they are passed to user-controlled functions), but happy to go through GHC proposal process.

I'd view this as a bug fix (overloaded strings should imply even generated strings are overloaded, if they are passed to user-controlled functions), but happy to go through GHC proposal process.

Ah, now that is a much solider point! Thanks.

BTW, I'm happy to get the work done, if there's acceptance that it really is a bug, and what it should do. Implementation notes follow (only read them if we're agreed it is a bug, if not I'll transfer them to a proposal).

There's still an open question of whether the fromString is injected only when you have OverloadedStrings and RebindableSyntax (and thus a user-controlled fail), or always for OverloadedStrings - my view is likely only when both are enabled. This would then be consistent with not desugaring pattern matches to fromString, since patError isn't user-controlled.

My fix would be that getFailFunction and failFunction in RnExpr.hs should be changed so that if OverloadedStrings is enabled then the fail_op would be fail . fromString rather than just fail as it is now.

The notes look ok to me. I do think a (small) proposal is the right way to proceed.

Proposal raised at https://github.com/ghc-proposals/ghc-proposals/pull/168

assigned to @shayne-fletcher-da

Proposal was accepted. Patch submitted at https://phabricator.haskell.org/D5251

Trac metadata

Trac field	Value
Differential revisions	- → D5251

Updated revision to Diff 18486.

mentioned in commit 8a4edd15

closed

changed milestone to %8.8.1

Trac metadata

Trac field	Value
Resolution	Unresolved → ResolvedFixed

added ghc proposal label

added Pnormal label