Opened 4 years ago

Closed 14 months ago

#5412 closed bug (fixed)

dataTypeConstrs gives unhelpful error message

Reported by: NeilMitchell Owned by: nomeata
Priority: normal Milestone: 7.6.2
Component: libraries/base Version: 7.1
Keywords: Cc: ndmitchell@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description (last modified by simonpj)

Take the following program:

import Data.Data
foo = dataTypeConstrs (dataTypeOf (0 :: Int))
-- raises: *** Exception: Data.Data.dataTypeConstrs

The implementation of dataTypeConstrs has the type available as a string at this point, so could easily say:

Exception: Data.Data.dataTypeConstrs is not supported for Int, 
           as it is not an algebraic data type

Attachments (4)

messages.patch.gz (1005 bytes) - added by klangner 14 months ago.
Patch
0001-Better-error-message-for-function-with-DataType-para.patch (3.1 KB) - added by klangner 14 months ago.
0001-Better-error-message.-Ticket-5412.patch (1.0 KB) - added by klangner 14 months ago.
0001-Fixed-error-messages.-Ticket-5412.patch (10.5 KB) - added by klangner 14 months ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 4 years ago by simonpj

  • Description modified (diff)

Good idea! Could you send a patch? Thanks!

Simon

comment:2 Changed 3 years ago by igloo

  • Milestone set to 7.6.1

comment:3 Changed 2 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:4 Changed 14 months ago by klangner

  • difficulty set to Unknown
  • Owner set to klangner

Changed 14 months ago by klangner

Patch

comment:5 Changed 14 months ago by klangner

  • Status changed from new to patch

comment:6 Changed 14 months ago by nomeata

  • Owner klangner deleted
  • Status changed from patch to new

Dear Krzysztof,

thanks for the patch. Note that SPJ made a more elaborate proposal in the ticket, namely printing the name of the type with the error message. Do you think you can update your ticket to print a message similar to what Simon proposed?

comment:7 Changed 14 months ago by nomeata

  • Owner set to klangner

comment:8 Changed 14 months ago by klangner

I can try :-)

I can create the following message:
Data.Data.dataTypeConstrs is not supported for Prelude.Int, as it is not an algebraic data type

As you can see there is Prelude.Int not Int.
This name is returned by dataTypeName.

I don't know how to get the name without Prelude prefix.

Changed 14 months ago by klangner

comment:9 Changed 14 months ago by klangner

  • Status changed from new to patch

comment:10 Changed 14 months ago by nomeata

  • Owner changed from klangner to nomeata

I just checked the API as well, and there does not seem to exist a clean way of getting the unqualified name. But as this is just an error message, I think it’s fine.

I’ll validate and push.

comment:11 Changed 14 months ago by nomeata

  • Owner changed from nomeata to klangner

Wait: The second patch only modifies dataTypeConstrs while the first patch changes the error message of several functions. Oversight?

Also, I’d add a “.” at the end of the message, after all, it is a full sentence.

comment:12 Changed 14 months ago by klangner

Yes you are right. The problem is that this kind of message can't be used in other places. So I was not sure if I should change other functions as well.
On the other hand it would be nice to improve some other error messages since probably sooner or later this will return as new ticket.

How do you think would be better?

  1. Change only function from this ticket description
  2. Change only messages where ADT is expected (4 places) and leave other messages intact.
  3. Try to improve other error messages in this module as well.

Third option will require more work and I'll probably need some help with the message texts.

comment:13 Changed 14 months ago by nomeata

Go for 3! The message will always be of the kind „Data.Data.... is not supported for ..., as it is not ...“

comment:14 Changed 14 months ago by klangner

Ok. I fixed 8 previous messages.
But I'm not sure what to do with this code:

Case 1.

instance Data Char where
  toConstr x = mkCharConstr charType x
  gunfold _ z c = case constrRep c of
                    (CharConstr x) -> z x
                    _ -> error "Data.Data.gunfold(Char)"
  dataTypeOf _ = charType

This instance is defined for Char type. So it is not possible to pass wrong type here. But I guess there still could be a problem with constructor representation.
There are similar instances for other data types as well. Lots of them.

Case 2.
In function: repConstr exception is thrown when parameters have different data types. So there is a need for another message. I would say something like:
"Data.Data.repConstr requires the same data type for both parameters."
or similar.

comment:15 Changed 14 months ago by nomeata

Case 1.
A better error message here would be "Data.Data.gunfold: Constructor " ++ show c ++ " is not of type Char". But if you don’t feel like working on that class of error message right now you can concentrate on the others and skip these for now.

Case 2.
Sounds good. Or maybe "Data.Data.repConstr: The given ConstrRep does not fit to the given DataType", possibly extended with actually showing both parameters, or using dataTypeName.

Changed 14 months ago by klangner

comment:16 Changed 14 months ago by klangner

Ok. Done.

comment:17 Changed 14 months ago by nomeata

  • Owner changed from klangner to nomeata

Looks good; validating right now...

comment:18 Changed 14 months ago by Joachim Breitner <mail@…>

In d0b74cac0b0ab5371d15b6f73c0e627b41c3a152/base:

Improve error messages for partial functions in Data.Data

This closes: #5412

comment:19 Changed 14 months ago by nomeata

  • Resolution set to fixed
  • Status changed from patch to closed

Validated and pushed. Thanks for your first contribution to GHC, and hoping to receive more of them!

Note: See TracTickets for help on using tickets.