riscv-software-src / riscof

BSD 3-Clause "New" or "Revised" License
63 stars 40 forks source link

Unorthodox Python in riscof template #71

Closed gsmecher closed 1 year ago

gsmecher commented 1 year ago

The template code (riscof_model.py) contains some benign but possibly bogus Python:

    def __init__(self, *args, **kwargs):
        sclass = super().__init__(*args, **kwargs)

        # [...]

        return sclass

The __init__ function is not supposed to return anything, on pain of TypeError. Quoth the data model docs:

Because new() and init() work together in constructing objects (new() to create it, and init() to customize it), no non-None value may be returned by init(); doing so will cause a TypeError to be raised at runtime.

I am guessing the threatened TypeError isn't emitted unless we're using mypy (which we aren't). However, it's possible there's something gnarlier happening behind the scenes and this code is intentional.

Overall, though, I think this is a bug. Because it's in template code, it tends to proliferate:

https://github.com/stnolting/neorv32-riscof/blob/main/plugin-neorv32/riscof_neorv32.py#L63 https://github.com/olofk/serv/blob/main/verif/plugin-sail_cSim/riscof_sail_cSim.py#L30

...which makes it worth tidying up.

Thank you for all your hard work.

gsmecher commented 1 year ago

Overall, though, I think this is a bug.

Confirmed - this code does nothing, and the capture and return of sclass should be removed.