[Refectoring] Remove Prism from type checking/compiler error handling #7111
Conversation
SeungheonOh
force-pushed
the
seungeonoh/6160
branch
from
50fa9b6 to
4efc5d1
Compare
SeungheonOh
force-pushed
the
seungeonoh/6160
branch
from
4efc5d1 to
9f4efa0
Compare
SeungheonOh
force-pushed
the
seungeonoh/6160
branch
from
9f4efa0 to
aa59c4c
Compare
|
@effectfully 0 conflicts with #7110 :D |
SeungheonOh
changed the title
SeungheonOh
force-pushed
the
seungeonoh/6160
branch
2 times, most recently
from
2819978 to
65675c1
Compare
| tryError :: MonadError e m => m a -> m (Either e a) | ||
| tryError a = (Right <$> a) `catchError` (pure . Left) | ||
| {-# INLINE tryError #-} | ||
|
|
There was a problem hiding this comment.
This probably should've been a separate PR
There was a problem hiding this comment.
It's a minor enough improvement, it's uncontroversial and even if it accidentally gets reverted who cares. I believe it's perfectly OK to lump such minor improvements into larger PRs.
There was a problem hiding this comment.
This looks like a step in the right direction. I think in future we should also get rid of MonadError constraints and just use Either/ExceptT directly and provide some automation for converting between different monad/error types, so that we don't keep writing all that modifyError blahblahblahblahblahblah boilerplate.
| @@ -236,33 +226,36 @@ uplcOptimise = | |||
| UnsafeOptimise -> id | |||
| in fmap PLC.runQuoteT | |||
| . _Wrapped | |||
| -- . modifyError (PIR.PLCError . PLC.FreeVariableErrorE) | |||
| ( MonadError err m -- Kind/type checking can fail | ||
| , AsTypeError err term uni fun ann -- with a 'TypeError'. | ||
| type MonadKindCheck error term uni fun ann m = | ||
| ( MonadError error m -- Kind/type checking can fail |
There was a problem hiding this comment.
At this point is there even benefit to keeping MonadError here? Can it be just Either? Can we simply move this functionality into TypeCheckT?
Anyway, that's probably is best to investigate separately, so feel free to ignore it here.
There was a problem hiding this comment.
I'd be happy to kill MonadError as well. It's probably best in a separate PR however. I'll make an issue.
| type MonadTypeCheckPir uni fun ann m = | ||
| ( MonadTypeCheck (PIR.Error uni fun ann) (Term TyName Name uni fun ()) uni fun ann m | ||
| -- , AsTypeErrorExt err uni ann -- Plutus IR has additional type errors, see 'TypeErrorExt'. | ||
| -- TODO: TypeErrorExt should really be the actual extension of PLC.TypeError |
There was a problem hiding this comment.
Aha, so this is where we can have different errors.
Commented out code btw.
| tryError :: MonadError e m => m a -> m (Either e a) | ||
| tryError a = (Right <$> a) `catchError` (pure . Left) | ||
| {-# INLINE tryError #-} | ||
|
|
There was a problem hiding this comment.
It's a minor enough improvement, it's uncontroversial and even if it accidentally gets reverted who cares. I believe it's perfectly OK to lump such minor improvements into larger PRs.
SeungheonOh
force-pushed
the
seungeonoh/6160
branch
from
65675c1 to
01970e6
Compare
closes #6160
Removed all prisms from type checking and compiler error handling. All functions previously threw unrealized error types were given minimal error type that can cover all kinds of error the function throws. Function calls within a function that throws different error type now requires changing error type using
modifyErrormanually.hoist-error seems like a viable option at first, but all we need is
Control.Monad.Except.modifyErrorsince error-throwing function calls are always made within another MonadError.hoistErroris just a more general version ofmodifyError, and in this case we don't need extra generality!Changes made in this PR arguably increases verbosity as this requires manually lifting error types of each function calls.