2374: Add error message for ambiguous type variables in inferred contexts - #2410
Conversation
| let constraintTypeVars = nub . foldMap (unknownsInType . fst) . filter ((`notElem` solved) . snd) $ zip (constraintArgs con) [0..] | ||
| when (any (`notElem` unsolvedTypeVars) constraintTypeVars) $ do | ||
| throwError . onErrorMessages (replaceTypes currentSubst) . errorMessage $ NoInstanceFound con | ||
| throwError . onErrorMessages (replaceTypes currentSubst) . errorMessage $ AmbiguousTypeVariables ident generalized con |
There was a problem hiding this comment.
Whoops; this code doesn't confirm that there are multiple instances of the constraint typeclass in existence. It seems like the strictly most correct response would be:
No instances -> NoInstanceFound
One instance -> Try to solve the type signature with that instance's type argument
Two instances -> AmbiguousTypeVariables
I'm not sure I have the expertise for that just yet, though.
There was a problem hiding this comment.
this will be handled elsewhere, so you don't need to worry about it here. An ambiguous type means something else, where a type variable appears in a constraint, but not in the body of the type, so it can't be inferred.
|
What you have looks correct to me. Thanks! Sent from my iPhone
|
| go TUnknown{} = True | ||
| go _ = False | ||
| renderSimpleErrorMessage (AmbiguousTypeVariables ident t (Constraint nm ts _)) = | ||
| paras [ line $ markCode (showIdent ident) ++ " doesn't have a type declaration, so it was inferred to be of type: " |
There was a problem hiding this comment.
I would omit this line, since it will just duplicate the information from the value declaration level (... in declaration x ...) anyway. In fact, I'd remove the Ident from the SimpleErrorMessage constructor.
|
Looks great so far, thanks! Please see my one comment, and update |
… since it's redundant with the enclosing compiler error's ident
| renderSimpleErrorMessage (AmbiguousTypeVariables t (Constraint nm ts _)) = | ||
| paras [ line "Inferred type" | ||
| , markCodeBox $ typeAtomAsBox t | ||
| , line " has constraints, but we don't know what type values they require. Please add an explicit type declaration for: " |
There was a problem hiding this comment.
Sorry to be picky, but can this be just "The inferred type ... has type variables which are not mentioned in the body of the type. Consider adding a type annotation"? Thanks.
There was a problem hiding this comment.
No problem; picky is good. Incoming.
| go TUnknown{} = True | ||
| go _ = False | ||
| renderSimpleErrorMessage (AmbiguousTypeVariables t (Constraint nm ts _)) = | ||
| paras [ line $ "The inferred type " ++ (markCode (prettyPrintTypeAtom t)) ++ " has type variables which are not mentioned in the body of the type. Consider adding a type annotation." |
There was a problem hiding this comment.
Sorry, one more 😄 :
Please put the type on its own line, indent it, and use prettyPrintType not prettyPrintTypeAtom. This will make it more consistent with other errors which display full types. The problem right now is that you risk it spilling over multiple lines.
Thanks!
| go _ = False | ||
| renderSimpleErrorMessage (AmbiguousTypeVariables t _) = | ||
| paras [ line "The inferred type" | ||
| , indent $ line $ markCode $ prettyPrintType t |
There was a problem hiding this comment.
I just realized this should use typeAsBox, since this way, the indentation gets messed up if the type takes up multiple lines.
I can put together a PR for this later, unless you want to take a look @bbqbaron ?
There was a problem hiding this comment.
Happy to; I'll set a reminder for tomorrow.
This adds ambiguous inferred type errors for 1.0. How does the error message look? It's my first time mucking about in the compiler; did I infer correctly when to throw the error?