Introduce GhciMonad and generalize types of functions in GHCi.UI
AcceptedPublic

Authored by watashi on Dec 10 2018, 2:18 PM.

Details

Summary

Introduce GhciMonad, which is bascially GhcMonad + HasGhciState.
Generalize the commands and help functions defined in GHCi.UI so they
can be used as both GHCi a and InputT GHCi a.

The long term plan is to move reusable bits to ghci library and make it
easier to build a customized interactive ui which carries customized state
and provides customized commands.

Most changes are trivial in this diff by relaxing the type constraint or
add/remove lift as necessary. The non-trivial changes are:

  • Change HasGhciState to GhciMonad and expose it.
  • Implementation of reifyGHCi.
Test Plan
./validate
watashi created this revision.Dec 10 2018, 2:18 PM
bgamari accepted this revision.Dec 11 2018, 12:54 PM

The long term plan is to move reusable bits to ghci library and make it
easier to build a customized interactive ui which carries customized state
and provides customized commands.

This sounds like a great direction to move in.

Patch looks good to me.

This revision is now accepted and ready to land.Dec 11 2018, 12:54 PM

I'd like to understand a bit more about the plan here. In general it's not necessary to overload everything in order to use the functions in a different monad - we can lift the functions instead. What is it that forces the overloading? Perhaps it's something to do with the structure of the interpreter, if we want to add more commands then the interpreter is calling back into our code, so we need to allow a different monad to be threaded through?

Oh, and in general I think InputT has infested much more of the GHCi code than it really should, it would be nice to restrict it to a much smaller area.

watashi added a comment.EditedDec 14 2018, 12:50 PM

What is it that forces the overloading?

Right now, nothing, we can always change GhciMonad m => m a to GHCi a and use lift when necessary.
It may help prevent GHCi a being changed to InputT GHCi a unnecessarily.
The code may look a little bit cleaner if we have a customized MyGhci (IO MyState -> InputT GHCi a) where we will have a lot of lift . lift otherwise.

Perhaps it's something to do with the structure of the interpreter, if we want to add more commands then the interpreter is calling back into our code, so we need to allow a different monad to be threaded through?

Yes, we din't change the type of PromptFunction or runOneCommand for this reason. We probably want to change the type GHCiState to be parameterized as GHCiState m in coming diffs.

in general I think InputT has infested much more of the GHCi code than it really should, it would be nice to restrict it to a much smaller area.

Agree. We were able to change a lot of InputT GHCi a to GhciMonad m => m a in this diff. If we are not sure about the type class change, we can at least restrict it to GHCi a.

What is it that forces the overloading?

Right now, nothing, we can always change GhciMonad m => m a to GHCi a and use lift when necessary.
It may help prevent GHCi a being changed to InputT GHCi a unnecessarily.

So with this method we get to change all the InputT GHCi a to GhciMonad m => m a. But I'd rather just see InputT GHCi a removed from the parts of GHCi that don't need it.

My question "what forces the overloading" could be rephrased as "What can't you do if you leave things as they are?". My intuition is that we want to make it possible to use GHCi as a library, in a custom front end that has its own monad. But we don't necessarily need to overload the whole of the GHCi API in order to do that - we can just lift the GHCi functions when calling them. So what is it that requires this overloading? If it's just convenience then I'm not sure it's worth it.

The code may look a little bit cleaner if we have a customized MyGhci (IO MyState -> InputT GHCi a) where we will have a lot of lift . lift otherwise.

I didn't follow that, could you give more detail?

We can surely go with GHCi instead of GhciMonad m in this diff, and revise it in the future should it becomes necessary.

1

Suppose we are implementing a customized ghci with some customized state, with GhciMonad we can have:

data MyState = MyState { foo :: Int, ghciState :: GHCiState }
newtype MyGhci a = MyGhci (IORef MyState -> Ghc a)
instnace GhciMonad MyGhci where
  ...

and use both functions of type m a and InputT m a directly; otherwise we have to define it as something like

data MyState = MyState { foo :: Int }
type MyGhci a = ReaderT (IORef MyState) (InputT GHCi a)

and use lift $ lift (_ :: m a) and lift (_ :: InputT m a) instead.
Changing the type of a function from GhcMonad g => g a to GHCi a or from GHCi a to InputT GHCi a will also more likely result in a change in callsite.

2

If we want to provide more callbacks like prompt :: [String] -> Int -> GHCi SDoc or possibly shutdownHook :: GHCi (),
with GhciMonad, we can change the type to m SDoc or m () and access the customized state;
otherwise, it's still to possible, but we need to explicitly capture the IORef MyState instead, like

mkCallback :: MyGhci a -> MyGhci (GHCi a)
mkCallback f = do
  stateRef <- ask
  return $ runReaderT stateRef $ runInputTWithPrefs defaultPrefs defaultSettings f

@simonmar Let me know what you think.