Add Error variants#115
Conversation
acl-cqc
left a comment
acl-cqc
left a comment
There was a problem hiding this comment.
Generally yes, but I think the API could be better (and maybe even less change if we keep the string)....
| show (BracketErr msg) = show msg | ||
| show (RemainingNatHopes hs) = unlines ("Expected to work out values for these holes:":((" " ++) <$> hs)) | ||
| show (NeedToKnow end) = unlines ["I wanna know what:", ' ':show end,"is."] | ||
| show (Both err1 err2) = unlines [show err1,""," AND WORSE","",show err2] |
There was a problem hiding this comment.
funny to read as the AND WORSE is, it's not necessarily true is it? There's no ordering or reason for the second to be worse than the first, is there? Should we just go with "AND ALSO" ?
| | ThunkLeftUnders String | ||
| | BracketErr BracketErrMsg | ||
| | RemainingNatHopes [String] | ||
| | NeedToKnow End |
There was a problem hiding this comment.
Why not make this take a Set of Ends?
|
|
||
| mkYield :: String -> S.Set End -> Free sig () | ||
| mkYield desc es = thTrace ("Yielding in " ++ desc ++ "\n " ++ show es) $ Yield (AwaitingAny es) (\_ -> trackM ("woke up " ++ desc) >> Ret ()) | ||
| mkYield :: ErrorMsg -> String -> S.Set End -> Free sig () |
There was a problem hiding this comment.
If NeedToKnow took a set of ends, you could drop this ErrorMsg param from mkYield and have it construct a NeedToKnow using the set it's given.
You might then keep the string, this seems useful as a hint as to where in the code we are blocked.
| ++ "":"Dynamic set is":(show <$> M.keys (dynamicSet ctx)) ++ ["Try writing more types! :-)"] | ||
| ++ "":"Dynamic set is":(show <$> M.keys (dynamicSet ctx)) | ||
| ++ "":["Try writing more types! :-)"]) | ||
| err |
There was a problem hiding this comment.
Whether you store err in Yield and use that here, or build NeedToKnow ends here, I don't really mind....
|
Couldn't agree more! Much nicer now |
|
Oh!! I hadn't actually realized that quite that much of the PR will disappear....i.e. that Both and NeedToKnow are no longer used at all. Sorry for repeated rework. I guess at this point it does look like smuggling these in will be the right thing, i.e. if they are actually used??? |
2 participants
Add an error message to
Yieldto print when typechecking ends with us getting stuck.