x-heep
x-heep copied to clipboard
Python: `<string_var> in ("<string>")` should be `<string_var> == "<string>"`
This issue affects the following files (that I noticed):
- hw/core-v-mini-mcu/peripheral_subsystem.sv.tpl
- util/mcu_gen.py
For some reason, the Python in operator is being used to compare a string variable with a string value, rather than using == directly.
These are correct:
s in ("foo", "bar", "baz")
s in ("foo", )
s in ["foo"]
s == "foo"
These are not:
s in "foo"
s in ("foo")
s in (("foo"))
Specifically, "foo" in ("foolish") will yield True since "foo" is a substring of "foolish".
(Note that simply adding parentheses to something won't create a sequence, but adding brackets or a trailing comma will.)
As a result, adding two peripherals with a similar name (e.g. "gpio" and "gpio_improved") can result in one of the peripherals being included twice, or result in the peripheral being included even when it is disabled in mcu_cfg.hjson.
I imagine the use of in was justified for comparing a string against multiple possible values, but for single values it should be replaced with ==.
(Or, if there really is a reason to use in instead of ==, the right-hand side MUST be a list/tuple, e.g. ("foo",) with a , at the end.)
Also, I don't understand the reason for iterating on peripherals.items(). It would make more sense to do either
% if peripherals['gpio']['is_included'] == 'yes':
or
% if 'gpio' in peripherals:
% if peripherals['gpio']['is_included'] == 'yes':
or
% if 'gpio' in peripherals and peripherals['gpio']['is_included'] == 'yes':
depending on whether or not the 'gpio' key is ensured to exist and what you want to do when it doesn't, and remove the % for peripheral in peripherals.items(): line.
(My understanding is that all keys are expected to exist, because it they don't then the % else: part tying the signals to zero will never be added.)