I agree that good error messages are crucial. I also agree that it can be hard for new feature designers to anticipate all the ways things might go wrong, and you've hit several bumps here. I've filed https://gitlab.haskell.org/ghc/ghc/-/issues/18466, which observes how painful this can be and suggests concrete improvements to error messages. I think it will be easy to implement these changes. Thanks for bringing the problem up! Richard

On Jul 17, 2020, at 8:36 AM, Simon Peyton Jones via ghc-devs wrote:

Thanks Erik. Error message quality is crucial, as you say.

But it's a very hard thing to measure, because it's a matter of human judgement. For example, you suggest

| I would like to suggest that a prerequesite for merging of new features would | be that it provides good error messages and more importantly does not provide | wrong or misleading error messages like the one above.

That is an excellent goal. But I don't know how to verify whether a patch meets that goal. GHC has several thousand regression tests; if the error message changes that is part of the patch, and is subject to code review. But it's harder to review the effect on test cases we'd never thought of, like yours.

The only feasible way I know is for GHC's users to submit bug reports, showing a bad error message, with a way to reproduce it in a small test case, and a suggestions for what would be a better one. That way, hopefully one of GHC's contributors will find a way to fix it.

Might you do that for your example (which I do not yet understand)? That's would be really helpful. It can take a little work to distil a small test case, but well-characterised bug reports are a major way in which GHC's users can contribute directly to improving GHC's quality.

Thanks

Simon

| -----Original Message----- | From: ghc-devs On Behalf Of Erik de Castro | Lopo | Sent: 17 July 2020 05:11 | To: ghc-devs@haskell.org | Subject: Error messages | | Hi all, | | Although it was many years ago I did spend soem time working on GHC | and I do know what a thankless task it is. I made a compliant about | GHC error messages on an internal Slack channel and Mortiz encouraged | me to repeat it here. | | I am incredibly happy about the quality of error messges for older | more standard parts of Haskell, probably most things in Haskell98 | and Haskell2010. | | By way of contrast, error messages for some newer parts of Haskell are | incredibly poor. In fact, for the new parts, the error rmessages are | often wrong, just defaulting to error messages for older parts of | Haskell. | | As an example (open source code in a public GH repo): | | src/Cardano/DbSync/StateQuery.hs:87:44: error: | • Data constructor not in scope: | QueryAnytime | :: QueryHardFork xs0 (Interpreter xs0) | -> Query | (CardanoBlock TPraosStandardCrypto) | (Interpreter (CardanoEras TPraosStandardCrypto)) | • Perhaps you want to add ‘QueryAnytime’ to the import list | in the import of | ‘Ouroboros.Consensus.HardFork.Combinator.Ledger.Query’ | (src/Cardano/DbSync/StateQuery.hs:49:1-116). | | | 87 | queryHistoryInterpreter connInfo (point, QueryAnytime | GetInterpreter) | | The suggestion is that I need to import `QueryAnytime` but just 20 line | above I | have: | | import Ouroboros.....Query (QueryAnytime (..), QueryHardFork | (GetInterpreter)) | | The problem is that `QueryAnytime` is defined as a pattern synonym. I | have only | the tinest amount of experience using pattern synonyms and that error | message | is not really useful. | | I would like to suggest that a prerequesite for merging of new features | would | be that it provides good error messages and more importantly does not | provide | wrong or misleading error messages like the one above. | | Erik | -- | ---------------------------------------------------------------------- | Erik de Castro Lopo | https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.mega | - | nerd.com%2F&data=02%7C01%7Csimonpj%40microsoft.com%7Cb97ca052f89f4363 | 73c508d82a076974%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63730555861 | 2145985&sdata=BbxTaOA9jhXbEX%2BmgZcwa0AjkZv8H58cgn0sCXgUwCU%3D&re | served=0 | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.has | kell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc- | devs&data=02%7C01%7Csimonpj%40microsoft.com%7Cb97ca052f89f436373c508d | 82a076974%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637305558612145985 | &sdata=yPnFMY3iqnBT8%2FMQTWiVg3aE1Ya%2BWxyqfjszc3XOMLw%3D&reserve | d=0 _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

It is interesting that Eric choose this particular example, as it is one of the few types of errors where I usually find GHC's suggestions for fixes to be quite useful---I am talking about the errors where you forgot to import something (or you mistype it's name) and GHC suggests that you import it or use a slightly different name. In general, I think it's better if error messages stick to describing the problem GHC is having, and avoid recommending solutions. The reason I say this is that in general we don't know what someone is trying to do and you need that to suggest the correct fix. In particular, often when I make a mistake it is because I have the wrong understanding of something, be it an API or Haskell itself, and GHC can't know what's in my head. For example, Eric says that he imported `QueryAnitime` but the import has `(..)` after it, which in Haskell means import the *type* `QueryAnytime` and all its constructors, while the problem is that we are missing a *data constructor* or pattern synonym with that name. Now this could be due to a typo, or maybe he doesn't know how exactly that part of the language works, or maybe he was thinking of a similar constructor with a slightly different name, all of which would cause the same error but would require different fixes. One constructive improvement to this error might be to change the words "data constructor" to "data constructor or pattern synonym" as both of these live in the same namespace and they are not completely interchangable (e.g., `..` imports data constructors but not type synonyms). Otoh, I think it'd be quite hard to suggest anything much more sophisticated than what we already do, and for me personally, that particular suggestion is often useful. Iavor On Fri, Jul 17, 2020, 02:09 Ben Gamari wrote:

On July 17, 2020 12:10:33 AM EDT, Erik de Castro Lopo < mle+hs@mega-nerd.com> wrote:

...

Hi all,

Although it was many years ago I did spend soem time working on GHC and I do know what a thankless task it is. I made a compliant about GHC error messages on an internal Slack channel and Mortiz encouraged me to repeat it here.

I am incredibly happy about the quality of error messges for older more standard parts of Haskell, probably most things in Haskell98 and Haskell2010.

By way of contrast, error messages for some newer parts of Haskell are incredibly poor. In fact, for the new parts, the error rmessages are often wrong, just defaulting to error messages for older parts of Haskell.

As an example (open source code in a public GH repo):

src/Cardano/DbSync/StateQuery.hs:87:44: error: • Data constructor not in scope: QueryAnytime :: QueryHardFork xs0 (Interpreter xs0) -> Query (CardanoBlock TPraosStandardCrypto) (Interpreter (CardanoEras TPraosStandardCrypto)) • Perhaps you want to add ‘QueryAnytime’ to the import list in the import of ‘Ouroboros.Consensus.HardFork.Combinator.Ledger.Query’ (src/Cardano/DbSync/StateQuery.hs:49:1-116). | 87 | queryHistoryInterpreter connInfo (point, QueryAnytime GetInterpreter)

The suggestion is that I need to import `QueryAnytime` but just 20 line above I have:

import Ouroboros.....Query (QueryAnytime (..), QueryHardFork (GetInterpreter))

The problem is that `QueryAnytime` is defined as a pattern synonym. I have only the tinest amount of experience using pattern synonyms and that error message is not really useful.

I am not sure that I would call this error wrong or misleading. However, I also suspect that it could do more to help you. GHC is claiming it is looking for a data constructor because that is precisely what it is looking for: Pattern synonyms are supposed to behave identically to data constructors.

It sounds to me like Ouroboros.....Query should be exporting the QueryAnytime pattern bundled with the QueryAnytime type. If this were the case then your import would work as you expect.

In principle GHC could suggest this (e.g. in the case where a module exports a type `T`, separately a pattern `T`, and the user imports the former but not the latter). However, this is in a way a strange suggestion as it would be suggesting a change in a module other than the module currently being compiled. This could be quite confusing since it may reveal internal implementation details of a library that the user is not supposed to be exposed to.

This is really more of a code style lintthan it is a compiler error.

...

I would like to suggest that a prerequesite for merging of new features would be that it provides good error messages and more importantly does not provide wrong or misleading error messages like the one above.

Indeed this is something that we consider during review. However, it is also very difficult to predict how features might fail, especially when interacting with other features. As Simon suggests, the best way to improve things here is to open tickets when you see an error that doesn't look as good as it could be.

Cheers,

- Ben

_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs