Bug 66

Summary: a nmigen "Object" is needed that acts like a multiply-inheritable python class
Product: Libre-SOC's first SoC Reporter: Luke Kenneth Casson Leighton <lkcl>
Component: Source CodeAssignee: Luke Kenneth Casson Leighton <lkcl>
Status: CONFIRMED ---    
Severity: enhancement CC: libre-soc-bugs, programmerjake, whitequark
Priority: ---    
Version: unspecified   
Hardware: PC   
OS: Linux   
See Also: https://bugs.libre-soc.org/show_bug.cgi?id=422
NLnet milestone: NLnet.2019.02.012 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:
child tasks for budget allocation: The table of payments (in EUR) for this task; TOML format:
Bug Depends on:    
Bug Blocks: 62    

Description Luke Kenneth Casson Leighton 2019-04-19 09:33:57 BST
Record is inadequate, not being multiply-inheritable, and requiring
declarations to be carried out through a non-pythonic method:
a list of tuples of signal width and direction.

this is much more intuitive:

class RecordTest:
   def __init__(self):
     self.r1 = RecordObject()
     self.r1.sig1 = Signal(16)

     self.r1.r2 = RecordObject()
     self.r1.r2.sig2 = Signal(16)

     self.r1.r3 = RecordObject()
     self.r1.r3.sig3 = Signal(16)

and, if RecordTest (or other class) is itself derived from RecordObject,
it may also be instantiated and assigned into another RecordObject,
to an arbitrary nested depth.

key features required:

* flattening
* eq (assignment) including with ArrayProxy
* iteration (single-argument)
* working well with nmigen ir/xfrm (Visitor code).
* preventing and prohibiting modification after first use

modification could be prevented using a class-decorator that sets
a flag whenever any function is used (except __init__), that is
then detected in __setattr__ and raises an exception
Comment 2 Jacob Lifshay 2019-04-19 09:42:16 BST
(In reply to Jacob Lifshay from comment #1)
> adapting the Data class could work well:
> https://salsa.debian.org/Kazan-team/simple-barrel-processor/blob/master/src/
> PipelineBuildingBlockAPIProposal.py#L24

one additional benefit, classes derived from Data interact transparently with Record instances.
Comment 3 Luke Kenneth Casson Leighton 2019-04-19 11:17:23 BST
(In reply to Jacob Lifshay from comment #2)
> (In reply to Jacob Lifshay from comment #1)
> > adapting the Data class could work well:
> > https://salsa.debian.org/Kazan-team/simple-barrel-processor/blob/master/src/
> > PipelineBuildingBlockAPIProposal.py#L24
> 
> one additional benefit, classes derived from Data interact transparently
> with Record instances.

showing that with a unit test would be good, to see how it works.
also, i think it would be a good idea to keep Data's public API as
closely mirroring nmigen Value as possible: a constructor instead of
create_data, iteration (__iter__) instead of members(), an eq function,
etc.

otherwise it breaks the principle of "least astonishment"
https://en.wikipedia.org/wiki/Principle_of_least_astonishment
Comment 4 Luke Kenneth Casson Leighton 2019-04-26 20:17:35 BST
http://bugs.libre-riscv.org/show_bug.cgi?id=64#c13
Comment 5 Luke Kenneth Casson Leighton 2020-12-04 12:28:57 GMT
adding whitequark to this one as it may be the case that work in nmigen "qualifies" for being funded by this donation.  to be verified as there were issues in (old) discussions that post-constructor-modification of the object (by inexperienced developers) would cause "surprises"