Fundamental inconsistency in dataclass initializtion: MVE

rebcabin commented 1 year ago

There is a fundamental inconsistency between LPython and CPython. I do not know how to make a single Python file with the following features that passes both. This issue is a minimization and clarification of Issue #2031, but not exactly a duplicate.

Consider

from lpython import (dataclass, i32)

@dataclass
class Foo:
    string : str

@dataclass
class Bar :
    foo : Foo
    il : list[i32]

bar : Bar = Bar(Foo('bar'), [1, 2, 3])

This passes CPython, but not LPython:


# CPYTHON

(lp) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)─────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@Golf37:s001)─┐
└─(19:11:35 on brian-lasr ✖ ✹ ✭)──> PYTHONPATH='../../src/runtime/lpython' python issue2045.py                                                        1 ↵ ──(Mon,Jun26)─┘

# LPYTHON

(lp) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)─────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@Golf37:s001)─┐
└─(19:11:51 on brian-lasr ✖ ✹ ✭)──> ~/CLionProjects/lpython/src/bin/lpython -I. issue2045.py                                                              ──(Mon,Jun26)─┘
semantic error: `foo` must be initialized with an instance of Foo
  --> issue2045.py:11:5
   |
11 |     foo : Foo
   |     ^^^^^^^^^

ok, so we'll add that initializer. Now it fails CPython but not LPython:

from lpython import (dataclass, i32)

@dataclass
class Foo:
    string : str

@dataclass
class Bar :
    foo : Foo = Foo('foo')
    il : list[i32]

bar : Bar = Bar(Foo('bar'), [1, 2, 3])

run:


# CPYTHON

(lp) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)─────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@Golf37:s001)─┐
└─(19:16:19 on brian-lasr ✖ ✹ ✭)──> PYTHONPATH='../../src/runtime/lpython' python issue2045.py                                                        2 ↵ ──(Mon,Jun26)─┘
Traceback (most recent call last):
  File "/Users/brian/CLionProjects/lpython/lasr/LP-pycharm/issue2045.py", line 8, in <module>
    class Bar :
  File "/Users/brian/CLionProjects/lpython/src/runtime/lpython/lpython.py", line 55, in dataclass
    return py_dataclass(arg)
  File "/Users/brian/mambaforge/envs/lp/lib/python3.10/dataclasses.py", line 1185, in dataclass
    return wrap(cls)
  File "/Users/brian/mambaforge/envs/lp/lib/python3.10/dataclasses.py", line 1176, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash,
  File "/Users/brian/mambaforge/envs/lp/lib/python3.10/dataclasses.py", line 1025, in _process_class
    _init_fn(all_init_fields,
  File "/Users/brian/mambaforge/envs/lp/lib/python3.10/dataclasses.py", line 546, in _init_fn
    raise TypeError(f'non-default argument {f.name!r} '
TypeError: non-default argument 'il' follows default argument

# LPYTHON

(lp) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)─────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@Golf37:s001)─┐
└─(19:16:36 on brian-lasr ✖ ✹ ✭)──> ~/CLionProjects/lpython/src/bin/lpython -I. issue2045.py                                                          1 ↵ ──(Mon,Jun26)─┘
(lp) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)─────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@Golf37:s00

Ok, so we'll add the initializer. Now it still fails CPython, and there seems to be no fix because lpython does not actually export field and default_factory:

from lpython import (dataclass, i32,
                     field, default_factory)

@dataclass
class Foo:
    string : str

@dataclass
class Bar :
    foo : Foo = Foo('foo')
    il : list[i32] = field(default_factory=list)

bar : Bar = Bar(Foo('bar'), [1, 2, 3])

run:


# CPYTHON

(lp) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)─────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@Golf37:s001)─┐
└─(19:20:10 on brian-lasr ✖ ✹ ✭)──> PYTHONPATH='../../src/runtime/lpython' python issue2045.py                                                        1 ↵ ──(Mon,Jun26)─┘
Traceback (most recent call last):
  File "/Users/brian/CLionProjects/lpython/lasr/LP-pycharm/issue2045.py", line 1, in <module>
    from lpython import (dataclass, i32,
ImportError: cannot import name 'field' from 'lpython' (/Users/brian/CLionProjects/lpython/src/runtime/lpython/lpython.py)

# LPYTHON

(lp) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)─────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@Golf37:s001)─┐
└─(19:20:22 on brian-lasr ✖ ✹ ✭)──> ~/CLionProjects/lpython/src/bin/lpython -I. issue2045.py                                                          1 ↵ ──(Mon,Jun26)─┘
(lp) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)─────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@Golf37:s001)─┐
└─(19:20:27 on brian-lasr ✖ ✹ ✭)──>

Thirumalai-Shaktivel commented 1 year ago

The following code can be used as a workaround:

from lpython import (dataclass, i32)

@dataclass
class Foo:
    string : str

@dataclass
class Bar :
    il : list[i32]
    foo : Foo = Foo('')

bar : Bar = Bar([1, 2, 3], Foo('bar'))

Do you think the above code violates your program design? (if so, we need to try other workaround or fix the actual bug)

rebcabin commented 1 year ago

I'm not so much worried about workarounds as I am about fixing bugs. My program and my work is all about finding bugs in LPython so that we can ship LPython to customers. I am trying to break LPython. I am trying to pretend to be a naive customer doing "reasonable" things and running into problems. The more bugs we fix by end of July, the happier customers will be.

rebcabin commented 1 year ago

The workaround was good, and I'm not blocked :) Still, this is a usability bug that will confuse customers (as it confused me :)

Smit-create commented 1 year ago

There are basically two issues here:

CPython requires all the data class members while creating an instance, but, LPython doesn't impose this strict constraint:
```
from lpython import (dataclass,)
```

@dataclass class Foo: string : str

bar : Foo = Foo()


This example will fail in CPython, but passes in LPython -- I think I know how to fix it here.

2. The second bug is what @rebcabin has reported in the very first example. CPython acts okay here with its policy of having initial value of all the structs members just because it has ignored the type annotations, and everything is fine at runtime.
```py
# CPython works even with this as it ignores type annotations/checking
from lpython import (dataclass, i32)

@dataclass
class Foo:
    string : str

@dataclass
class Bar :
    foo : Foo
    il : list[i32]

bar : Bar = Bar(32, [1, 2, 3])
print(bar.foo, bar.il)

LPython deals here a bit differently. I discussed the same with @certik some time ago, and the reason why LPython wants structs to be initialized is to avoid accidentally using unallocated structs (or their members).

lcompilers / lpython

Fundamental inconsistency in dataclass initializtion: MVE #2045