Opened 2 years ago

Last modified 11 months ago

#14341 patch bug

Show instance for TypeReps is a bit broken

Reported by: dfeuer Owned by:
Priority: normal Milestone: 8.6.1
Component: Core Libraries Version: 8.2.1
Keywords: Typeable Cc: RyanGlScott
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D4084, Phab:D5080
Wiki Page:

Description (last modified by dfeuer)

There are three problems.

  1. Showing typereps of tuples can produce unnecessary parentheses:
Prelude K T> typeRep @(Int, Maybe Bool)
(Int,(Maybe Bool))

The fix is trivial.

  1. Showing typereps of ticked (i.e., lifted) tuples and lists gives hard-to-read results, because it does not use the usual special syntax:
Prelude K T> typeRep @'(Int, Maybe Bool)
'(,) * * Int (Maybe Bool)

Prelude K T> typeRep @'[1,2,3]
': Nat 1 (': Nat 2 (': Nat 3 ('[] Nat)))

Fixing the lifted tuple case is trivial. Fixing the lifted list case is slightly less trivial, but not hard.

  1. Type operator applications are not shown infix.
Prelude K T> typeRep @(Maybe :*: Either Int)
:*: * Maybe (Either Int)

This is the hardest problem to fix, although it's probably not too terribly hard. See comment:1 for thoughts.

Change History (6)

comment:1 Changed 2 years ago by dfeuer

Actually, there's a third problem as well: type operators are not shown infix! The solution, I believe, is to add operator precedence to TyCon, and then to use something akin to Show derivation machinery in showTypeRep. I imagine we don't need to worry too much about showing these things efficiently, at least for the foreseeable future. SomeTypeRep has no Read instance, so there's no substantial risk of anyone trying to use its Show instance for anything other than error messages and interactive exploration.

comment:2 Changed 2 years ago by dfeuer

Description: modified (diff)

comment:3 Changed 13 months ago by PhilippD

There's also a problem with displaying partially applied tuple type constructors:

λ> typeRep @((,,))
()
λ> typeRep @((,,,,,,,) Int Word)
(Int,Word)

The problem seems to be that showTypeable doesn't check the arity of the constructor and just applies showArgs to the type arguments.

comment:4 Changed 13 months ago by bgamari

Differential Rev(s): Phab:D4084Phab:D4084, Phab:D5080
Milestone: 8.6.1
Status: newpatch

I whipped up Phab:D5080 which solves the correctness problem mentioned in comment:3 simply.

comment:5 Changed 11 months ago by Ben Gamari <ben@…>

In 846fe904/ghc:

Typeable: Only render saturated tuple types with tuple syntax

This isn't as efficient as it could be since it needs to compute the
kind of the type. However, this is `show` so there shouldn't be any
particular expectation of speed.

Fixes #14341.

Test Plan: Validate

Reviewers: hvr

Subscribers: monoidal, rwbarton, carter

GHC Trac Issues: #14341

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

comment:6 Changed 11 months ago by bgamari

There is still more than could be done here but at least the correctness issues are solved.

Note: See TracTickets for help on using tickets.