On July 17, 2020 12:10:33 AM EDT, Erik de Castro Lopo
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