Incorrect Read deriving for MagicHash constructors

Greetings,

A fellow in #haskell on freenode just discovered the following bug:

{-# LANGUAGE MagicHash #-}

data Note = A# deriving (Show, Read)

show works fine, but read doesn't:

*Main> show A#
"A#"
*Main> read (show A#)
*** Exception: Prelude.read: no parse

Probably not a surprising omission, and hopefully not difficult to fix.

Trac metadata

Trac field	Value
Version	7.0.2
Type	Bug
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Compiler
Test case
Differential revisions
BlockedBy
Related
Blocking
CC
Operating system
Architecture

changed weight to 5

added Tbug Trac import labels

ghci needs to be called with "-XMagicHash" for "show A#" to work and read needs to know the result type. Yet:

*Main> read "A#" :: Note
*** Exception: Prelude.read: no parse

changed milestone to %7.2.1

The difficulty here is that lexId in the library Text.Read.Lex doesn't recognise A# as a lexeme.

I can't see an obviously good workaround for this. The library can't change with different GHC flags. Alternatives:

• When GHC does 'Show' for a constructor A#, make it spit out some funny escape sequence like A_hash. (And dually for reading.) But that is no fun. The ASCII in the file would look odd to a human, and human-written stuff like A# would fail.
• Simply fix Text.Reader.Lex to recognise a postfix hash as a legal lexeme. The downside here is that lex ought to parse a+x#c as five single-character lexemes, so we'd be changing the behaviour away from H98.
• Like the previous idea, but make a copy of Text.Reader.Lex and invoke that when building derived Read instances. The original lex would stay unchanged. In effect we'd just have a new function lexWithTrailingHashes. Actually you would not need to copy all of it, by a long chalk; but there'd be a little duplication.

My suggestion would be to follow the third plan. Anyone who cares, feel free to express an opinion.

Once we've decided, executing the fix should be easy, so I'll milestone for 7.2

Simon

Generate a parser that recognises "A" followed by "#"? It would be too liberal (extra spaces allowed), but it would work, no?

Options 2 and 3 above are precisely that.

Replying to [ticket:5041#comment:49973 simonpj]:

Options 2 and 3 above are precisely that.

No, do it without modifying Text.Reader.Lex. Just generate a Read instance that recognises the two lexemes "A" followed by "#".

Attached file 5041.dpatch (download).

I've attached a patch that fixes this bug following Simon Marlow's suggestion.

mentioned in commit bf0d3df4

mentioned in commit fd44046f

closed

Fixed by

commit bf0d3df4d011bc93af28b195a97abfcd24b9e7d6
Author: simonpj <simonpj@cam-04-unx.europe.corp.microsoft.com>
Date:   Tue Apr 19 13:31:53 2011 +0100

    Fix Trac #5041: parse the trailing '#'
    
    This matters for constructors and field labels that
    have a trailing hash (MagicHash language extension).

 compiler/typecheck/TcGenDeriv.lhs |   31 ++++++++++++++++++-------------
 1 files changed, 18 insertions(+), 13 deletions(-)

Trac metadata

Trac field	Value
Resolution	Unresolved → ResolvedFixed
Test case	→ deriving/should_run/T5041

reopened

This doesn't look right to me:

{-# LANGUAGE MagicHash #-}

data Foo = Foo#
    deriving (Show, Read)

foo :: Foo
foo = read "Foo #" -- NOTE: Space between "Foo" and "#"

main :: IO ()
main = print foo

$ ./q
Foo#

Trac metadata

Trac field	Value
Resolution	ResolvedFixed → Unresolved

removed milestone

changed weight to 3

That's right. If you look at the discussion thread above we decided that allowing the space fixed your problem without making an invasive change to the libraries. It's too liberal as you note, but it's not getting in anyone's way (as far as I know). Until it does it's not worth the effort to fix it properly (unless someone wants to volunteer).

Still, fair enough, it's still wrong so I'll leave it open with milestone bottom

Simon

Trac metadata

Trac field	Value
Priority	normal → low

added deriving label

mentioned in issue #14918 (closed)

mentioned in commit d5577f44

added Plow label

λ❯ read (show A#) :: Note
A#
it :: Note

Seems to do the trick. Closing for now, can be reopened if needed.

closed

I have been reading the code generating Read instances and it is still bogus when MagicHash is used. Reproducing example:

{-# LANGUAGE MagicHash #-}

module Main where

import Text.Read

data T1 = A#              deriving (Show,Read)
data T2 = B# | C#         deriving (Show,Read)
data T3 = D# | D          deriving (Show,Read)
data T4 = E#              deriving (Show,Read)
data T5 = F# Int | F Int  deriving (Show,Read)
data T6 = G##             deriving (Show,Read)
data T7 = H## Int | H Int deriving (Show,Read)

main :: IO ()
main = do
  print (readMaybe "A#"     :: Maybe T1)
  print (readMaybe "B#"     :: Maybe T2)
  print (readMaybe "C#"     :: Maybe T2)
  print (readMaybe "D#"     :: Maybe T3)
  print (readMaybe "D"      :: Maybe T3)
  print (readMaybe "E#"     :: Maybe T4)
  print (readMaybe "F# 10"  :: Maybe T5)
  print (readMaybe "F 10"   :: Maybe T5)
  print (readMaybe "G##"    :: Maybe T6)
  print (readMaybe "H## 10" :: Maybe T7)
  print (readMaybe "H 10"   :: Maybe T7)

gives with HEAD:

Just A#
Nothing
Nothing
Nothing
Just D
Just E#
Just (F# 10)
Just (F 10)
Nothing
Nothing
Just (H 10)

There is a hack to support a single # for non nullary constructors and when there is only one nullary constructor.

reopened