[libre-riscv-dev] Very Subtle Synchronous nMigen Behavior

[Luke Kenneth Casson Leighton]

On Fri, Jun 19, 2020 at 1:31 AM Yehowshua <yimmanuel3 at gatech.edu> wrote:
>
> I will mention I won’t use wildcards in any code I commit ever.

urrr, thank you :)

the problem is not immediately apparent until you start developing
large codebases, importing from multiple sources (some your own, some
other people's libraries), and wish to navigate them.

for minerva, i found it was already seriously problematic to navigate
the codebase in order to track down which class came from which actual
file.  in addition, they actually had (may still have) a duplicate
class that was wildcard-imported in one location and *overridden with
a second*.  if they had explicitly named it the discrepancy would have
been caught fairly quickly.


in our case...

lkcl at fizzy:~/src/libresoc/soc/src/soc$ find . -name "*.py" | wc
    238     238    6862
lkcl at fizzy:~/src/libresoc/ieee754fpu$ find . -name "*.py" | wc
    319     319   12399

[side-note: both those codebases - on their own, not combined - exceed
the number of files in nmigen by 60 and 100% respectively.  in total
there are *seven hundred* source code files to navigate and we're
nowhere near finished]

can you imagine a new person - or anyone in fact - looking at one
file, where there are 50 import statements, all of them wildcards,
having *no idea* what was imported by what, trying to navigate the
codebase and understand it?

this is the ALU pipeline main_stage.py imports:

    from soc.alu.pipe_data import ALUInputData, ALUOutputData
    from ieee754.part.partsig import PartitionedSignal
    from soc.decoder.power_enums import InternalOp
    from soc.alu.maskgen import MaskGen
    from soc.alu.rotl import ROTL

    from soc.decoder.power_fields import DecodeFields
    from soc.decoder.power_fieldsn import SignalBitRange

you know exactly what comes from where, and you know, from these
declarations, that if you want to see what PartitionedSignal does, you
go to the directory "ieee754/part" and open the file "partsig.py".
that might confuse you for a while if you didn't know that ieee754 was
a separate library, however you'd soon work it out, and, after a
while, you'd begin to link in your mind, "from soc" means "the
libre-soc main codebase" and "from ieee754" means "the ieee754
library", "from nmigen" means "go to the nmigen library and look
there" and so on.

however if the file looks like this:

    from soc.alu.pipe_data import *
    from ieee754.part.partsig import *
    from soc.decoder.power_enums import *
    from soc.alu.maskgen import *
    from soc.alu.rotl import *

    from soc.decoder.power_fields import *
    from soc.decoder.power_fieldsn import *

now you have no idea from scanning the alu/main_stage.py where
PartitionedSignal even *comes* from.  you now have to open *SEVEN
FILES* and, worse, SCAN THEIR ENTIRE CONTENTS just to find out which
one contains that class.

using vim "tag" capability (ctags -R then pressing ctrl-]) will not
help you because PartitionedSignal is in the *ieee754* library... not
the main SoC codebase (unless you are editing from a really *really*
low level root directory and running ctags -R there).

not only that but this particular example page alone, if the practice
of wildcard imports were deployed systematically throughout the
codebase, the cascading effect of one module wildcard-importing
another which continues in a recursive chain would result in SEVERAL
THOUSAND unnecessary imports being pulled in.

i've seen instances in nmigen where a single wildcard import brought
in over 70 objects, only 2 of which were actually needed (the exact
same reasoning above applies to nmigen just as much.  nmigen having
internal wildcard imports *genuinely* interferes with our ability to
develop our project as it makes it very difficult to navigate)

with a *seventy thousand* line codebase (and likely to be at least
double that by the time we're done), there's just... we would be
seriously, *seriously* making our lives absolute hell.

l.