lpython icon indicating copy to clipboard operation
lpython copied to clipboard

Request better error message for `dict` and `set`

Open rebcabin opened this issue 2 years ago • 3 comments

I often make the following stupid mistake:

from lpython import (i32, dataclass)

@dataclass
class FullFormValue:
    list_i32 : list[i32]
    string   : str

foo : dict[str, FullFormValue]

DEAD_LIST : list[i32] = [-1]  # Issue

ttype    : FullFormValue = FullFormValue(DEAD_LIST, 'dimensions')
contents : FullFormValue = FullFormValue([1, 2], '')

foo = {'ttype'   , ttype,
       'contents', contents}

The error message is accurate, but could be more helpful:

/Users/brian/CLionProjects/lpython/src/bin/python /Users/brian/CLionProjects/lpython/lasr/LP-pycharm/issue2034.py 
semantic error: All Set values must be of the same type for now
  --> /Users/brian/CLionProjects/lpython/lasr/LP-pycharm/issue2034.py:15:7 - 16:28
   |
15 |    foo = {'ttype'   , ttype,
   |          ^^^^^^^^^^^^^^^^^^^...
...
   |
16 |           'contents', contents}
   | ...^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 

LPython probably knows that the right-hand side should be a dict, so it might be better if LPython told me of a type mismatch between the left-hand and right-hand sides, that the right-hand side should be a dict and not a set. Instead, it tells me that my set is wrong. That's correct, of course, but it leads me in the wrong direction. If LPython allowed heterogeneous sets, it wouldn't even report that error!

rebcabin avatar Jun 26 '23 22:06 rebcabin

    foo = {'ttype', ttype,
        'contents', contents}

Here, the value (in the assignment statement) is visited first and ASR is constructed for that. That's why, the set members are checked for type match. After constructing the value ASR LPython visits the target. Here is another example that throws the error you were expecting:

    foo = {'ttype', 'ttype',
        'contents', 'contents'}
semantic error: Type mismatch in assignment, the types must be compatible
  --> examples/expr2.py:36:5
   |
36 |     foo = {'ttype', 'ttype',
   |     ^^^ type mismatch ('dict[str, FullFormValue]' and 'set[str]')
   |
36 |        foo = {'ttype', 'ttype',
   |              ^^^^^^^^^^^^^^^^^^...
...
   |
37 |            'contents', 'contents'}
   | ...^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type mismatch ('dict[str, FullFormValue]' and 'set[str]')


Note: if any of the above error or warning messages are not clear or are lacking
context please report it to us (we consider that a bug that must be fixed).

Thirumalai-Shaktivel avatar Jun 27 '23 09:06 Thirumalai-Shaktivel

I'm not sure if it is possible to show some kind of hint only for the Dict and Set assignment. So, that it would be easy for the user to debug the code. I need to investigate this.

Thirumalai-Shaktivel avatar Jun 27 '23 09:06 Thirumalai-Shaktivel

I think this issue would get fixed when we support error recovery mechanism. With the error recovery, I think LPython would/can print two errors as follows:

semantic error: All Set values must be of the same type for now
   |
15 |    foo = {'ttype'   , ttype,
   |          ^^^^^^^^^^^^^^^^^^^...
...
   |
16 |           'contents', contents}

semantic error: Type mismatch in assignment, the types must be compatible
  --> examples/expr2.py:36:5
   |
36 |     foo = {'ttype', 'ttype',
   |     ^^^ type mismatch ('dict[str, FullFormValue]' and 'set[str]')
   |
36 |        foo = {'ttype', 'ttype',
   |              ^^^^^^^^^^^^^^^^^^...
...
   |
37 |            'contents', 'contents'}
   | ...^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type mismatch ('dict[str, FullFormValue]' and 'set[str]')

ubaidsk avatar Mar 06 '24 20:03 ubaidsk