Bug 906

Summary:	change HDL code to not use type annotations even for dataclasses
Product:	Libre-SOC's first SoC	Reporter:	Jacob Lifshay <programmerjake>
Component:	Source Code	Assignee:	Jacob Lifshay <programmerjake>
Status:	RESOLVED FIXED
Severity:	enhancement	CC:	libre-soc-bugs, lkcl, programmerjake
Priority:	---
Version:	unspecified
Hardware:	Other
OS:	Linux
NLnet milestone:	NLnet.2021.02A.052.CryptoRouter	total budget (EUR) for completion of task and all subtasks:	0
budget (EUR) for this task, excluding subtasks' budget:	0	parent task for budget allocation:	771
child tasks for budget allocation:		The table of payments (in EUR) for this task; TOML format:	jacob=0

Description Jacob Lifshay 2022-08-10 08:59:57 BST

https://lists.libre-soc.org/pipermail/libre-soc-dev/2022-August/005183.html

type/other assertions are TBD, so should not be removed until we decide what to do. They're awaiting lkcl's response to:
https://lists.libre-soc.org/pipermail/libre-soc-dev/2022-August/005208.html
If we decide to not remove assertions, then we should revert:
https://git.libre-soc.org/?p=nmutil.git;a=commitdiff;h=d72880215d54123709c8c02b86446b5a752c27c9

DONE:
* nmigen-gf/src/nmigen_gf/hdl/cldivrem.py
* nmutil/src/nmutil/prefix_sum.py
* nmutil/src/nmutil/lut.py
* ieee754fpu/src/ieee754/part_swizzle/swizzle.py
TODO:
* soc/src/soc/fu/div/experiment/goldschmidt_div_sqrt.py

Plan:
* DONE: create decorator function in nmutil that reads __slots__ and creates eq, hash, compare, and repr, as well as allowing freezing like dataclasses. don't include generation of init, the user has to write that so people can see how the fields are set, making it more obvious to python beginners.

* WIP: replace all dataclasses in hdl with the decorator and add __init__ methods.

use the decorator like:
@plain_data() # always use paren, makes impl simpler
class A:
    __slots__ = ("a", "b")
    def __init__(self, a, b):
        self.a = a
        self.b = b

# Usage demo:
assert repr(A(1, 2)) == "A(a=1, b=2)" # repr works
assert A(3, 4) == A(3, 4) # eq works

# Another example:
@plain_data(unsafe_hash=True, frozen=True)
class B:
    __slots__ = ("c", "d")
    def __init__(self, c, d):
        self.c = c
        self.d = d

# Usage demo:
d = {B(3, 4): 1, B(5, 6): 2} # works because hash and frozen set

Comment 1 Jacob Lifshay 2022-08-11 06:06:25 BST

started adding the @plain_data() decorator and tests:
https://git.libre-soc.org/?p=nmutil.git;a=commit;h=80d0f36d1fa88f28f6f8709a5f19043ef602272b

Comment 2 Jacob Lifshay 2022-08-12 08:11:42 BST

Finished implementing plain_data.

It's nicer to use than dataclass if you have a frozen instance and you write your own __init__, because it still allows field writes until __init__ finishes running:

@dataclass(frozen=True)
class MyData:
    a: int
    b: int

    def __init__(self, a, b):
        # insert custom logic here
        # we have to use object.__setattr__ to bypass the frozen-type logic
        object.__setattr__(self, "a", a)
        object.__setattr__(self, "b", b)

@plain_data(frozen=True)
class MyData:
    __slots__ = "a", "b"

    def __init__(self, a, b):
        # insert custom logic here
        self.a = a
        self.b = b

Converted prefix_sum.py to use plain_data instead of dataclass:
https://git.libre-soc.org/?p=nmutil.git;a=blob;f=src/nmutil/prefix_sum.py;h=65f2b33eaf079889590d5aa13ffb6fdb28f1628c;hb=fcbd2f816210cd48879f05ac6d342f93409e03b4#l13

Comment 3 Jacob Lifshay 2022-08-12 09:00:52 BST

I finished replacing dataclass in most the rest of the code (nmutil, nmigen-gf, and ieee754fpu), only place left is goldschmidt_div_sqrt.

I'm not replacing dataclass in nmigen, sby, or sv_binutils.py. that covers everything I have checked out on my computer.

Comment 4 Luke Kenneth Casson Leighton 2022-08-12 11:04:49 BST

(In reply to Jacob Lifshay from comment #3)
> I finished replacing dataclass in most the rest of the code (nmutil,
> nmigen-gf, and ieee754fpu), only place left is goldschmidt_div_sqrt.
> 
> I'm not replacing dataclass in nmigen, sby,

hell no, they're "outside the HDL remit" and i explained that the
strictness of dataclasses is strongly appropriate for formal correctness.

> or sv_binutils.py.

that's down to dmitry, the case for using dataclass there is...
[annoyingly-but-i-got-over-it] justifiable.  now that plain_data
exists it *might* be useable... 

> that covers everything I have checked out on my computer.

fantastic, thank you jacob.

Comment 5 Jacob Lifshay 2022-08-16 07:46:14 BST

I added nmutil.plain_data.fields and nmutil.plain_data.replace utility functions, based on the dataclasses.fields and dataclasses.replace functions.

I finished converting all dataclasses usages in the HDL.

Comment 6 Jacob Lifshay 2022-08-16 07:49:23 BST

idk if this is the right parent task, lkcl, please fill out the payment details as appropriate.

Comment 7 Luke Kenneth Casson Leighton 2022-08-16 10:46:43 BST

(In reply to Jacob Lifshay from comment #5)
> I added nmutil.plain_data.fields and nmutil.plain_data.replace utility
> functions, based on the dataclasses.fields and dataclasses.replace functions.
> 
> I finished converting all dataclasses usages in the HDL.

brilliant.  it maaay actually be appropriate to consider moving them
to nmigen, as one of the candidates for moving to nmigen is RecordObject,
augmenting it to use this.

(In reply to Jacob Lifshay from comment #6)
> idk if this is the right parent task, lkcl, please fill out the payment
> details as appropriate.

771? no - there's another place it can go, under the Horizon2020 grants.