Opened 3 years ago

Closed 7 months ago

#11786 closed bug (fixed)

Need -fno-print-explicit-runtime-reps to work on IfaceType, else RuntimeRep leaks

Reported by: simonpj Owned by:
Priority: highest Milestone: 8.6.1
Component: Compiler Version: 7.10.3
Keywords: LevityPolymorphism Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: #13275, #15181 Differential Rev(s): Phab:D4733
Wiki Page:

Description

After #11549, we are suppose not to show RuntimeRep to users, when -fno-print-explicit-runtime-reps is on. But we do:

:info ($)
Prelude> :i ($)
($) ::  forall (r :: GHC.Types.RuntimeRep) a (b :: TYPE r).
        (a -> b) -> a -> b
  	-- Defined in ‘GHC.Base’

This happens because the runtime-rep-suppression happens on Type, but for :info we use pprInfo which uses tyThingToIfaceDecl and then prints it. And tyThingToIfaceDecl doesn't know about the -fprint-explicit-runtime-reps flag; and I'd like to keep it that way.

We want to change pprType to convert to IfaceType and then print anyway.

I suppose that this means that the pretty-printer for IfaceType needs to know about this runtime-rep suppression. That's a bit tiresome, but not impossible, especially since there is no shadowing to worry about.

Change History (18)

comment:1 Changed 3 years ago by simonpj

Indeed, since

  • we want :type do do deep-instantiation (#11376), and
  • I think we don't generalise over RuntimeRep variables,

the only time -fno-print-explicit-runtime-reps now has an effect (except perhaps in debug output) is in :info; and that is precisely when it doesn't work!

comment:2 Changed 3 years ago by simonpj

Test T11549 demonstrates this

comment:3 Changed 3 years ago by Simon Peyton Jones <simonpj@…>

In f2a2b79f/ghc:

Deeply instantiate in :type

See Trac #11376 and
 Note [Deeply instantiate in :type] in TcRnDriver

Sadly this showed up one new problem (Trac #11786) and one opportunity
(Trac #11787), so test T11549 is now marked expect-broken on these two.

comment:4 Changed 3 years ago by bgamari

For the record: The transition away from the Type pretty-printer to IFaceType is being tracked as #11660. I have a patch in progress for this, although have lacked time to finish it.

comment:5 Changed 3 years ago by bgamari

Milestone: 8.2.1
Resolution: fixed
Status: newclosed

This has been fixed along with #11660.

comment:6 Changed 17 months ago by sighingnow

This bug still exists in ghc-HEAD.

With or without :set -fprint-explicit-runtime-reps, we both have

Prelude> :t ($)
($) :: (a -> b) -> a -> b
Prelude> :i ($)
($) ::
  forall (r :: GHC.Types.RuntimeRep) a (b :: TYPE r).
  (a -> b) -> a -> b
        -- Defined in ‘GHC.Base’
infixr 0 $

This behavior is inconsistent with the description in user guide: https://downloads.haskell.org/~ghc/master/users-guide/using.html#ghc-flag--fprint-explicit-runtime-reps

comment:7 Changed 17 months ago by goldfire

Milestone: 8.2.18.6.1
Priority: normalhighest
Resolution: fixed
Status: closednew

comment:8 Changed 16 months ago by RyanGlScott

Keywords: LevityPolymorphism added

comment:9 Changed 16 months ago by bgamari

Hmm, interesting. I wonder what regressed; the logic to instantiate runtime-reps certainly exists (see IfaceType.defaultRuntimeRepVars).

comment:10 in reply to:  3 Changed 16 months ago by sighingnow

Owner: set to sighingnow

I have looked into this ticket. The feature :set -fprint-explicit-runtime-reps doesn't work in ghc 8.0.1/8.0.2/8.2.2/8.4.2/head.


In IfaceType.hs, we use pprIfaceSigmaType to pretty print a type:

pprIfaceSigmaType :: ShowForAllFlag -> IfaceType -> SDoc
pprIfaceSigmaType show_forall ty
  = ppr_iface_forall_part show_forall tvs theta (ppr tau)
  where
    (tvs, theta, tau) = splitIfaceSigmaTy ty

The kind signatures are not feed into eliminateRuntimeRep and defaultRuntimeRepVars. The forall (r :: GHC.Types.RuntimeRep) a (b :: TYPE r) is pretty printed by pprUserIfaceForAll (invoked by ppr_iface_forall_part).

pprUserIfaceForAll :: [IfaceForAllBndr] -> SDoc
pprUserIfaceForAll tvs
   = sdocWithDynFlags $ \dflags ->
     -- See Note [When to print foralls]
     ppWhen (any tv_has_kind_var tvs
             || any tv_is_required tvs
             || gopt Opt_PrintExplicitForalls dflags) $
     pprIfaceForAll tvs
   where
     tv_has_kind_var (TvBndr (_,kind) _) = not (ifTypeIsVarFree kind)
     tv_is_required = isVisibleArgFlag . binderArgFlag

So, for ($), we always print ($) :: forall (r :: GHC.Types.RuntimeRep) a (b :: TYPE r) (a -> b) -> a -> b.


As for why :t and :i print different result, in f2a2b79fa8d1c702b17e195a70734b06625e0153, we instantiate deeply for :type, then :t and :i have different behaviour for ($). If we revert this commit, :t ($) will produce same result as :i ($).


In ticket:11376#comment:34, Simon wrote:

OK, well we can both live with

  • :type var prints the original type of var, whereas :type expr typechecks, instantiates, and re-generalises the type of expr.

But how long will it be until someone posts a bug report complaining that :t (blah) is different from :t blah?

Maybe not long, but we can just point to the user manual. Having two commands is a pain when you can get the second by adding parens to the first.

Now we just need someone to do it.

I will try this and optimistically assign this ticket to myself.

Last edited 16 months ago by sighingnow (previous) (diff)

comment:11 Changed 16 months ago by sighingnow

Differential Rev(s): Phab:D4733
Status: newpatch

comment:12 Changed 16 months ago by sighingnow

I have just noticed that ticket:11376#comment:34 has already been implemented by Phab:2136, via :type +v.

I have revised Phab:D4733 to fix -fprint-explicit-runtime-reps only.

comment:13 Changed 15 months ago by Ben Gamari <ben@…>

In 40db277f/ghc:

Fix `print-explicit-runtime-reps` (#11786).

By fixing splitting of IfaceTypes in splitIfaceSigmaTy.

Test Plan: make test TEST="T11549 T11376 T11786"

Reviewers: goldfire, bgamari

Subscribers: rwbarton, thomie, carter

GHC Trac Issues: #11786, #11376

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

comment:14 Changed 15 months ago by bgamari

Resolution: fixed
Status: patchclosed

comment:15 Changed 7 months ago by simonpj

Owner: sighingnow deleted
Resolution: fixed
Status: closednew

comment:16 Changed 7 months ago by simonpj

Yikes. I have just wasted 30 minutes (in Core dumps) figuring out why a type printed as

forall a b. Eq a => blah

didn't match a term

/\ a. \(d :: Eq a). blah

Turns out that the real type is forall a. Eq a => forall b. blah, but the pretty printer silently moves the foralls to the top. This is done in splitIfaceSigmaTy in the patch in comment:13.

It might be just arguable to move the foralls around for user types. E.g in test ghci025 we now report

return :: Monad m => a -> m a

whereas previously it was

return :: Monad m => forall a. a -> m a

But we definitely should not do this in Core printouts.

I also noticed that in the output for ghci025 we have

c3 :: forall a b a. C a b => a -> b

The repeated 'a' is terrible!

So I'm re-opening this ticket to ask: do we want to swizzle the foralls to the top, even in user printout? And if so, should we provide a way to switch off this behaviour? E.g perhaps -fprint-explicit-foralls can print the foralls exactly where they are. At the moment it's horribly misleading.

Last edited 7 months ago by simonpj (previous) (diff)

comment:17 Changed 7 months ago by simonpj

Another oddity is that elminiateRunTimeRep (which works recursively) is called by pprIfaceSigmaTy, which is called from ppr_sigma which is called by ppr_ty which is also recursive.

This can't be right. I think eliminateRuntimeRep should only be called at top level

comment:18 Changed 7 months ago by goldfire

Resolution: fixed
Status: newclosed

It seems a new ticket makes more sense: #16320.

Note: See TracTickets for help on using tickets.