Offer way to retrieve GHC warnings

[mvdan]

See #8. This isn't a regression, since this was never really feasible.

[gelisam]

(I didn't remove the "help wanted" tag because help is no longer wanted, but because that tag is redundant; I welcome help on all the issues, not just those which have the "help wanted" tag!)

[phlummox]

Okay. So it looks as if this involves amending mayFail, which currently looks like this:

mayFail :: MonadInterpreter m => m (Maybe a) -> m a
mayFail action =
    do
        maybe_res <- action
        --
        es <- modifySessionRef ghcErrListRef (const [])
        --
        case (maybe_res, null es) of
            (Nothing, True)  -> throwM $ UnknownError "Got no error message"
            (Nothing, False) -> throwM $ WontCompile (reverse es)
            (Just a, _)      -> return a

And in the "Just" case, if the error list is not null, then there were warnings. The simple fix would be to throw those warnings, but that forces people to do a catch, even when the compilation succeeded. So I assume that instead, we have to return an (a, [GhcError]), or something like that.

And then, all the functions that call mayFail -- which is a lot of them -- either have to bubble that upward (a backwards-breaking API change), or provide a new "with warnings" variant (e.g. in addition to, say, typeOf, we'd also have ... typeOfWithWarnings, or something?)

I'm happy to do a pull request, if anyone can confirm whether the latter (adding WithWarnings variants) is a reasonable approach. (Or has alternative suggestions.)

[gelisam]

Okay. So it looks as if this involves amending mayFail

Thanks for investigating!

The simple fix would be to throw those warnings, but that forces people to do a catch, even when the compilation succeeded.

That doesn't work because we should be able to get both the successful result and the warnings, but if you throw the warnings, then the successful result is no longer available.

we have to return an (a, [GhcError]), or something like that.

That's more like it! Since I assume that most callers won't care about the warnings, how about emitting the warnings as a WriterT effect instead of an explicit tuple?

all the functions that call mayFail -- which is a lot of them -- either have to bubble that upward (a backwards-breaking API change), or provide a new "with warnings" variant (e.g. in addition to, say, typeOf, we'd also have ... typeOfWithWarnings, or something?)

That would certainly work, but that doesn't seem very composable. Changing the type of all those functions so that they produce a WriterT would be much more composable: one function, decide whether to ignore the warnings with evalWriterT or to keep them with runWriterT. A few concerns, however:

1. WriterT is notoriously-leaky, better to use a State-based implementation.
2. For backwards compatibility, let's accumulate warning as part of the existing InterpreterT rather than as a separate effect. That is, let's add a new function getAndClearWarnings which returns all the warnings which have been accumulated since the last getAndClearWarning call.
3. This API is backwards-compatible, but takes more and more memory in the common case in which getAndClearWarnings is never called. Let's add an accumulateWarnings option which defaults to False.

[phlummox]

Agreed - amending the monad would be much more composable. I had thought there were issues with using WriterT and StateT with MonadMask, hence my not suggesting that - but apparently they were resolved a couple of years ago in exceptions version 0.10.0. Neat :)

Anyway, I'll get onto this once I have some current projects out of the way (hopefully in a couple of weeks' time.)

cheers!

[gelisam]

I had thought there were issues with using WriterT and StateT with MonadMask, hence my not suggesting that - but apparently they were resolved a couple of years ago in exceptions version 0.10.0. Neat :)

I'm glad you like exception-0.10, that's the one version of exceptions I'm responsible for :)