error-messages icon indicating copy to clipboard operation
error-messages copied to clipboard

do desugaring yields confusing errors when the argument count is wrong

Open elaforge opened this issue 3 years ago • 9 comments

module BadError where

{-
I get:
    Couldn't match expected type ‘() -> Either String Int’
        with actual type ‘Either String Int’
    ...
  |
6 |     x <- do_thing arg2
  |     ^^^^^^^^^^^^^^^^^^

This makes it seem like "do_thing arg2" is short one argument, but it's not!
'confusing' has an undeclared argument ().
-}

confusing :: Int -> String -> () -> Either String Int
confusing arg1 arg2 = do
    x <- do_thing arg2
    -- ... much stuff
    return $ arg1 + 2
    where
    do_thing :: String -> Either String Int
    do_thing = undefined

{-
In this case I think I was misled by the highlighting, it highlights "do_thing
arg2".  But the "in the expression" is correct, it fingers the entire do
expression.  When desugared, it seems that this is an artifact of the error
highlighting, since the below highlights the entire expression:
-}

-- desugared :: Int -> String -> () -> Either String Int
-- desugared arg1 arg2 =
--     do_thing arg2 >>= \x -> return $ arg1 + 2
--     where
--     do_thing :: String -> Either String Int
--     do_thing = undefined

elaforge avatar Feb 18 '22 03:02 elaforge

I think it thinks (->) () is the monad? I had no good examples off hand, but I do think the function monad in general leads to all sorts of bad errs. Wondering what a fix other than draconian getting rid of it might look like.

Ericson2314 avatar Feb 18 '22 17:02 Ericson2314

In the specific error here, is the problem simply the highlighted code at the bottom of the message? That is, if it was more apparent that the entire do was problematic, would that have been better?

goldfirere avatar Feb 18 '22 19:02 goldfirere

It's that a missing argument on the toplevel function confusing instead gets (apparently) pinned on do_thing. The ideal thing would be for it to say the signature and declaration don't line up, like it does when you have multiple equations with inconsistent numbers of arguments, but due to currying this could be hard. After that, yes I think pointing at the entire do block would definitely be an improvement, since it wouldn't give the impression that we know where the error is, and here is the wrong spot.

Though I think the (->) instances lead to plenty of other bad errors, I think this particular case is not related to that. In general I've noticed that missing or extra args on functions that immediately go into a do block, can give confusing errors, I assume due to the desugaring. Presumably the error happens on desugared source and then ideally would need to be re-sugared in some way to put it back into the context of the original source. That sounds like it could be hard in general, but maybe some heuristics could catch simple cases like this.

Even without a do-block we have this:

f :: Int -> Int -> Int
f x = x * 2

It has a confusing error but it's classic currying. I wonder if a heuristic to try to detect this situation would also get the do-block one?

elaforge avatar Feb 18 '22 20:02 elaforge

Yes, I also tend to think that Monad ((->) env) is not to blame here. (Though I agree it may be blameworthy elsewhere.)

I'm still not sure about the original error on this ticket, though. The expected/actual part of the error seems very sensible to me -- though maybe there should be a note about the wrong number of arguments. And if the entire do block is blamed -- not just the first line -- then I think we're in OK territory. Do you agree?

GHC does not desugar before type checking: it analyzes the code the user wrote. So I don't think we need to worry about desugaring errors here.

goldfirere avatar Feb 21 '22 16:02 goldfirere

Yes, I agree the error "Couldn't match expected type () -> Either String Int with actual type Either String Int is reasonable, and the misleading part is where it points. Blaming the whole do-block would be a great improvement, and in fact is what happens on the desugared version. Sounds like my guess about desugaring was wrong, but still seems there's something a bit odd about how SrcPos extents get generated in a do block, and its interaction with currying.

elaforge avatar Feb 21 '22 20:02 elaforge

OK. So I think then we have a GHC bug we can submit. In the module

module Bug where

f :: Int -> Bool -> IO ()
f x = do
  print x
  stuff <- getLine
  putStrLn stuff

GHC blames print x for the type error. It should blame the whole do block.

Is that a good next step?

goldfirere avatar Feb 22 '22 17:02 goldfirere

OK yes so Monad ((->) env) is not actually the problem as it hasn't gotten as far as constraint solving (IIUC), but I still feel what is a problem is that it sees something like type of (do ...) = Bool -> IO () as a unification problem?

How are we to ensure it does better without some sort of backtracking? Forcing inference mode on do ... might help here, but would be rather pessimistic in other scenarios

Also I bring up this more general stuff because in general I have had issues with currying causing unhelpful:

m a = (b -> c)
---------------
m = (->) b

conclusions in GHC, and this feels like merely a special case of that.

Ericson2314 avatar Feb 22 '22 17:02 Ericson2314

I wrote https://github.com/ghc-proposals/ghc-proposals/discussions/487 for a crazy language (anti-)extension to make these errors go away.

that might be a genuinely useful restriction that, while kinda ad-hoc, may still be easier to understand than messing around checking/inferring.


Ultimately, I would want IDE mode to make the type checker to various guessing to try to minimize type errors --- while a user stares at an error, it can do it in the background, and then issue a notification if it thinks it can reduse the error by "shifting blame around".

That would be an absolutely killer feature, but it is performance intensive and also requires a huge engineering effort.

Ericson2314 avatar Feb 22 '22 17:02 Ericson2314

It's possible that the plan I'm proposing -- blame the entire do block, not just one statement -- is infeasible, but that's not clear yet. This is why I'd like to raise this problem to the GHC team so we can think about whether this is possible.

goldfirere avatar Feb 22 '22 18:02 goldfirere