Revise function arity mismatch errors involving TypeApplications
authorRyan Scott <ryan.gl.scott@gmail.com>
Tue, 22 Aug 2017 13:29:01 +0000 (09:29 -0400)
committerRyan Scott <ryan.gl.scott@gmail.com>
Tue, 22 Aug 2017 13:29:01 +0000 (09:29 -0400)
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

compiler/hsSyn/HsUtils.hs
compiler/typecheck/TcExpr.hs
compiler/typecheck/TcUnify.hs
testsuite/tests/typecheck/should_fail/T13902.hs [new file with mode: 0644]
testsuite/tests/typecheck/should_fail/T13902.stderr [new file with mode: 0644]
testsuite/tests/typecheck/should_fail/all.T

index 97ab76f..374fbe9 100644 (file)
@@ -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)
 
index 195ba01..801e58a 100644 (file)
@@ -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)
index b792f95..5136649 100644 (file)
@@ -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 (file)
index 0000000..73f34f2
--- /dev/null
@@ -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 (file)
index 0000000..c3d07ed
--- /dev/null
@@ -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
index 69e2e99..5fbbee0 100644 (file)
@@ -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, [''])