[commit: ghc] master: Revise function arity mismatch errors involving TypeApplications (8476097)
git at git.haskell.org
git at git.haskell.org
Tue Aug 22 14:56:35 UTC 2017
Repository : ssh://git@git.haskell.org/ghc
On branch : master
Link : http://ghc.haskell.org/trac/ghc/changeset/8476097609a2044e965157380aeb5d4882a71248/ghc
>---------------------------------------------------------------
commit 8476097609a2044e965157380aeb5d4882a71248
Author: Ryan Scott <ryan.gl.scott at gmail.com>
Date: Tue Aug 22 09:29:01 2017 -0400
Revise function arity mismatch errors involving TypeApplications
Summary:
Currently, whenever you apply a function to too many arguments and
some of those arguments happen to be visible type applications, the error
message that GHC gives is rather confusing. Consider the message you receive
when typechecking `id @Int 1 2`:
```
The function `id` is applied to three arguments,
but its type `Int -> Int` has only one
```
This is baffling, since the two lines treat the visible type argument `@Int`
differently. The top line ("applied to three arguments") includes `@Int`,
whereas the bottom line ("has only one") excludes `@Int` from consideration.
There are multiple ways one could fix this, which I explain in an addendum to
`Note [Herald for matchExpectedFunTys]`. The approach adopted here is to change
the herald of this error message to include visible type arguments, and to
avoid counting them in the "applied to n arguments" part of the error. The end
result is that the new error message for `id @Int 1 2` is now:
```
The expression `id @Int` is applied to two arguments,
but its type `Int -> Int` has only one
```
Test Plan: make test TEST=T13902
Reviewers: goldfire, austin, bgamari
Reviewed By: goldfire
Subscribers: rwbarton, thomie
GHC Trac Issues: #13902
Differential Revision: https://phabricator.haskell.org/D3868
>---------------------------------------------------------------
8476097609a2044e965157380aeb5d4882a71248
compiler/hsSyn/HsUtils.hs | 5 ++++-
compiler/typecheck/TcExpr.hs | 23 ++++++++++++++++------
compiler/typecheck/TcUnify.hs | 17 ++++++++++++++++
testsuite/tests/typecheck/should_fail/T13902.hs | 8 ++++++++
.../tests/typecheck/should_fail/T13902.stderr | 8 ++++++++
testsuite/tests/typecheck/should_fail/all.T | 1 +
6 files changed, 55 insertions(+), 7 deletions(-)
diff --git a/compiler/hsSyn/HsUtils.hs b/compiler/hsSyn/HsUtils.hs
index 97ab76f..374fbe9 100644
--- a/compiler/hsSyn/HsUtils.hs
+++ b/compiler/hsSyn/HsUtils.hs
@@ -20,7 +20,7 @@ which deal with the instantiated versions are located elsewhere:
module HsUtils(
-- Terms
- mkHsPar, mkHsApp, mkHsAppType, mkHsAppTypeOut, mkHsCaseAlt,
+ mkHsPar, mkHsApp, mkHsAppType, mkHsAppTypes, mkHsAppTypeOut, mkHsCaseAlt,
mkSimpleMatch, unguardedGRHSs, unguardedRHS,
mkMatchGroup, mkMatch, mkPrefixFunRhs, mkHsLam, mkHsIf,
mkHsWrap, mkLHsWrap, mkHsWrapCo, mkHsWrapCoR, mkLHsWrapCo,
@@ -178,6 +178,9 @@ mkHsApp e1 e2 = addCLoc e1 e2 (HsApp e1 e2)
mkHsAppType :: LHsExpr name -> LHsWcType name -> LHsExpr name
mkHsAppType e t = addCLoc e (hswc_body t) (HsAppType e t)
+mkHsAppTypes :: LHsExpr name -> [LHsWcType name] -> LHsExpr name
+mkHsAppTypes = foldl mkHsAppType
+
mkHsAppTypeOut :: LHsExpr GhcTc -> LHsWcType GhcRn -> LHsExpr GhcTc
mkHsAppTypeOut e t = addCLoc e (hswc_body t) (HsAppTypeOut e t)
diff --git a/compiler/typecheck/TcExpr.hs b/compiler/typecheck/TcExpr.hs
index 195ba01..801e58a 100644
--- a/compiler/typecheck/TcExpr.hs
+++ b/compiler/typecheck/TcExpr.hs
@@ -1188,7 +1188,7 @@ tcApp m_herald orig_fun orig_args res_ty
; (wrap_fun, args1, actual_res_ty)
<- tcArgs fun fun_sigma orig args
- (m_herald `orElse` mk_app_msg fun)
+ (m_herald `orElse` mk_app_msg fun args)
-- this is just like tcWrapResult, but the types don't line
-- up to call that function
@@ -1202,9 +1202,16 @@ tcApp m_herald orig_fun orig_args res_ty
mk_hs_app f (Left a) = mkHsApp f a
mk_hs_app f (Right a) = mkHsAppType f a
-mk_app_msg :: LHsExpr GhcRn -> SDoc
-mk_app_msg fun = sep [ text "The function" <+> quotes (ppr fun)
- , text "is applied to"]
+mk_app_msg :: LHsExpr GhcRn -> [LHsExprArgIn] -> SDoc
+mk_app_msg fun args = sep [ text "The" <+> text what <+> quotes (ppr expr)
+ , text "is applied to"]
+ where
+ what | null type_app_args = "function"
+ | otherwise = "expression"
+ -- Include visible type arguments (but not other arguments) in the herald.
+ -- See Note [Herald for matchExpectedFunTys] in TcUnify.
+ expr = mkHsAppTypes fun type_app_args
+ type_app_args = rights args
mk_op_msg :: LHsExpr GhcRn -> SDoc
mk_op_msg op = text "The operator" <+> quotes (ppr op) <+> text "takes"
@@ -1261,7 +1268,11 @@ tcArgs :: LHsExpr GhcRn -- ^ The function itself (for err msgs only)
tcArgs fun orig_fun_ty fun_orig orig_args herald
= go [] 1 orig_fun_ty orig_args
where
- orig_arity = length orig_args
+ -- Don't count visible type arguments when determining how many arguments
+ -- an expression is given in an arity mismatch error, since visible type
+ -- arguments reported as a part of the expression herald itself.
+ -- See Note [Herald for matchExpectedFunTys] in TcUnify.
+ orig_expr_args_arity = length $ lefts orig_args
go _ _ fun_ty [] = return (idHsWrapper, [], fun_ty)
@@ -1291,7 +1302,7 @@ tcArgs fun orig_fun_ty fun_orig orig_args herald
go acc_args n fun_ty (Left arg : args)
= do { (wrap, [arg_ty], res_ty)
<- matchActualFunTysPart herald fun_orig (Just (unLoc fun)) 1 fun_ty
- acc_args orig_arity
+ acc_args orig_expr_args_arity
-- wrap :: fun_ty "->" arg_ty -> res_ty
; arg' <- tcArg fun arg arg_ty n
; (inner_wrap, args', inner_res_ty)
diff --git a/compiler/typecheck/TcUnify.hs b/compiler/typecheck/TcUnify.hs
index b792f95..5136649 100644
--- a/compiler/typecheck/TcUnify.hs
+++ b/compiler/typecheck/TcUnify.hs
@@ -98,6 +98,23 @@ This is used to construct a message of form
The function 'f' is applied to two arguments
but its type `Int -> Int' has only one
+When visible type applications (e.g., `f @Int 1 2`, as in #13902) enter the
+picture, we have a choice in deciding whether to count the type applications as
+proper arguments:
+
+ The function 'f' is applied to one visible type argument
+ and two value arguments
+ but its type `forall a. a -> a` has only one visible type argument
+ and one value argument
+
+Or whether to include the type applications as part of the herald itself:
+
+ The expression 'f @Int' is applied to two arguments
+ but its type `Int -> Int` has only one
+
+The latter is easier to implement and is arguably easier to understand, so we
+choose to implement that option.
+
Note [matchExpectedFunTys]
~~~~~~~~~~~~~~~~~~~~~~~~~~
matchExpectedFunTys checks that a sigma has the form
diff --git a/testsuite/tests/typecheck/should_fail/T13902.hs b/testsuite/tests/typecheck/should_fail/T13902.hs
new file mode 100644
index 0000000..73f34f2
--- /dev/null
+++ b/testsuite/tests/typecheck/should_fail/T13902.hs
@@ -0,0 +1,8 @@
+{-# LANGUAGE TypeApplications #-}
+module T13902 where
+
+f :: a -> a
+f x = x
+
+g :: Int
+g = f @Int 42 5
diff --git a/testsuite/tests/typecheck/should_fail/T13902.stderr b/testsuite/tests/typecheck/should_fail/T13902.stderr
new file mode 100644
index 0000000..c3d07ed
--- /dev/null
+++ b/testsuite/tests/typecheck/should_fail/T13902.stderr
@@ -0,0 +1,8 @@
+
+T13902.hs:8:5: error:
+ • Couldn't match expected type ‘Integer -> Int’
+ with actual type ‘Int’
+ • The expression ‘f @Int’ is applied to two arguments,
+ but its type ‘Int -> Int’ has only one
+ In the expression: f @Int 42 5
+ In an equation for ‘g’: g = f @Int 42 5
diff --git a/testsuite/tests/typecheck/should_fail/all.T b/testsuite/tests/typecheck/should_fail/all.T
index 69e2e99..5fbbee0 100644
--- a/testsuite/tests/typecheck/should_fail/all.T
+++ b/testsuite/tests/typecheck/should_fail/all.T
@@ -451,6 +451,7 @@ test('T12373', normal, compile_fail, [''])
test('T13610', normal, compile_fail, [''])
test('T11672', normal, compile_fail, [''])
test('T13819', normal, compile_fail, [''])
+test('T13902', normal, compile_fail, [''])
test('T11963', normal, compile_fail, [''])
test('T14000', normal, compile_fail, [''])
test('T14055', normal, compile_fail, [''])
More information about the ghc-commits
mailing list