lib.wiring.connect: diagnostic when no connection made.
If a connect() call results in no connections being made, and it's because there were no outputs specified at all, issue an error. Tests enumerate cases per https://github.com/amaranth-lang/amaranth/pull/1153#issuecomment-1962810678. Co-authored-by: Catherine <whitequark@whitequark.org>
This commit is contained in:
parent
09029cdd91
commit
a586df89ad
|
@ -1371,6 +1371,7 @@ def connect(m, *args, **kwargs):
|
||||||
* For a given path, if any of the interface objects has an input port member corresponding
|
* For a given path, if any of the interface objects has an input port member corresponding
|
||||||
to a constant value, then the rest of the interface objects must have output port members
|
to a constant value, then the rest of the interface objects must have output port members
|
||||||
corresponding to the same constant value.
|
corresponding to the same constant value.
|
||||||
|
* When connecting multiple interface objects, at least one connection must be made.
|
||||||
|
|
||||||
For example, if :py:`obj1` is being connected to :py:`obj2` and :py:`obj3`, and :py:`obj1.a.b`
|
For example, if :py:`obj1` is being connected to :py:`obj2` and :py:`obj3`, and :py:`obj1.a.b`
|
||||||
is an output, then :py:`obj2.a.b` and :py:`obj2.a.b` must exist and be inputs. If :py:`obj2.c`
|
is an output, then :py:`obj2.a.b` and :py:`obj2.a.b` must exist and be inputs. If :py:`obj2.c`
|
||||||
|
@ -1420,10 +1421,15 @@ def connect(m, *args, **kwargs):
|
||||||
reasons_as_string)
|
reasons_as_string)
|
||||||
signatures[handle] = obj.signature
|
signatures[handle] = obj.signature
|
||||||
|
|
||||||
# Collate signatures and build connections.
|
# Connecting zero or one signatures is OK.
|
||||||
|
if len(signatures) <= 1:
|
||||||
|
return
|
||||||
|
|
||||||
|
# Collate signatures, build connections, track whether we see any input or output.
|
||||||
flattens = {handle: iter(sorted(signature.members.flatten()))
|
flattens = {handle: iter(sorted(signature.members.flatten()))
|
||||||
for handle, signature in signatures.items()}
|
for handle, signature in signatures.items()}
|
||||||
connections = []
|
connections = []
|
||||||
|
any_in, any_out = False, False
|
||||||
# Each iteration of the outer loop is intended to connect several (usually a pair) members
|
# Each iteration of the outer loop is intended to connect several (usually a pair) members
|
||||||
# to each other, e.g. an out member `[0].a` to an in member `[1].a`. However, because we
|
# to each other, e.g. an out member `[0].a` to an in member `[1].a`. However, because we
|
||||||
# do not just check signatures for equality (in order to improve diagnostics), it is possible
|
# do not just check signatures for equality (in order to improve diagnostics), it is possible
|
||||||
|
@ -1437,6 +1443,7 @@ def connect(m, *args, **kwargs):
|
||||||
# implied in the flow of each port member, so the signature members are only classified
|
# implied in the flow of each port member, so the signature members are only classified
|
||||||
# here to ensure they are not connected to port members.
|
# here to ensure they are not connected to port members.
|
||||||
is_first = True
|
is_first = True
|
||||||
|
first_path = None
|
||||||
sig_kind, out_kind, in_kind = [], [], []
|
sig_kind, out_kind, in_kind = [], [], []
|
||||||
for handle, flattened_members in flattens.items():
|
for handle, flattened_members in flattens.items():
|
||||||
path_for_handle, member = next(flattened_members, (None, None))
|
path_for_handle, member = next(flattened_members, (None, None))
|
||||||
|
@ -1499,6 +1506,8 @@ def connect(m, *args, **kwargs):
|
||||||
# There are no port members at this point; we're done with this path.
|
# There are no port members at this point; we're done with this path.
|
||||||
continue
|
continue
|
||||||
# There are only port members after this point.
|
# There are only port members after this point.
|
||||||
|
any_in = any_in or bool(in_kind)
|
||||||
|
any_out = any_out or bool(out_kind)
|
||||||
is_first = True
|
is_first = True
|
||||||
for (path, member) in in_kind + out_kind:
|
for (path, member) in in_kind + out_kind:
|
||||||
member_shape = member.shape
|
member_shape = member.shape
|
||||||
|
@ -1574,6 +1583,14 @@ def connect(m, *args, **kwargs):
|
||||||
out_path=(*out_path, index), in_path=(*in_path, index))
|
out_path=(*out_path, index), in_path=(*in_path, index))
|
||||||
assert out_member.dimensions == in_member.dimensions
|
assert out_member.dimensions == in_member.dimensions
|
||||||
connect_dimensions(out_member.dimensions, out_path=out_path, in_path=in_path)
|
connect_dimensions(out_member.dimensions, out_path=out_path, in_path=in_path)
|
||||||
|
|
||||||
|
# If no connections were made, and there were inputs but no outputs in the
|
||||||
|
# signatures, issue a diagnostic as this is most likely in error.
|
||||||
|
if len(connections) == 0 and any_in and not any_out:
|
||||||
|
raise ConnectionError(f"Only input to input connections have been made between several "
|
||||||
|
f"interfaces; should one of them have been flipped?")
|
||||||
|
|
||||||
|
|
||||||
# Now that we know all of the connections are legal, add them to the module. This is done
|
# Now that we know all of the connections are legal, add them to the module. This is done
|
||||||
# instead of returning them because adding them to a non-comb domain would subtly violate
|
# instead of returning them because adding them to a non-comb domain would subtly violate
|
||||||
# assumptions that `connect()` is intended to provide.
|
# assumptions that `connect()` is intended to provide.
|
||||||
|
|
|
@ -980,6 +980,65 @@ class ConnectTestCase(unittest.TestCase):
|
||||||
'(eq (sig q__a__0__0) (sig p__a__0__0))',
|
'(eq (sig q__a__0__0) (sig p__a__0__0))',
|
||||||
])
|
])
|
||||||
|
|
||||||
|
def test_connect_none(self):
|
||||||
|
# Connecting zero or more empty signatures is permitted as (a) it's not
|
||||||
|
# something you can write mistakenly out by hand, and (b) it permits
|
||||||
|
# generic code to expand to nothing without errors around edges.
|
||||||
|
m = Module()
|
||||||
|
connect(m)
|
||||||
|
|
||||||
|
def test_connect_empty(self):
|
||||||
|
m = Module()
|
||||||
|
connect(m, p=NS(signature=Signature({})))
|
||||||
|
|
||||||
|
def test_connect_empty_multi(self):
|
||||||
|
m = Module()
|
||||||
|
connect(m, p=NS(signature=Signature({})),
|
||||||
|
q=NS(signature=Signature({})))
|
||||||
|
|
||||||
|
def test_connect_one(self):
|
||||||
|
# Connecting just one signature should be allowed for the same reasons
|
||||||
|
# as above. (It's possible to forget an argument, but that stands out.)
|
||||||
|
m = Module()
|
||||||
|
connect(m, p=NS(signature=Signature({"a": Out(1), "b": In(1)}),
|
||||||
|
a=Signal(),
|
||||||
|
b=Signal()))
|
||||||
|
|
||||||
|
def test_connect_one_in_only(self):
|
||||||
|
# As above, even if there's only inputs.
|
||||||
|
m = Module()
|
||||||
|
connect(m, p=NS(signature=Signature({"a": In(1)}),
|
||||||
|
a=Signal()))
|
||||||
|
|
||||||
|
def test_connect_multi_in_only_fails(self):
|
||||||
|
# If we're only attempting to connect multiple inputs, we're not
|
||||||
|
# actually doing anything and it's most likely a mistake.
|
||||||
|
m = Module()
|
||||||
|
with self.assertRaisesRegex(ConnectionError,
|
||||||
|
r"^Only input to input connections have been made between several interfaces; "
|
||||||
|
r"should one of them have been flipped\?$"):
|
||||||
|
connect(m,
|
||||||
|
p=NS(signature=Signature({"a": In(1), "b": In(8)}),
|
||||||
|
a=Signal(),
|
||||||
|
b=Signal(8)),
|
||||||
|
q=NS(signature=Signature({"a": In(1), "b": In(8)}),
|
||||||
|
a=Signal(),
|
||||||
|
b=Signal(8)))
|
||||||
|
|
||||||
|
def test_connect_multi_some_in_pairs(self):
|
||||||
|
# Connecting matching inputs is an allowed no-op if there are also
|
||||||
|
# actual input-output connections to be made. See
|
||||||
|
# https://github.com/amaranth-lang/amaranth/pull/1153#issuecomment-1962810678
|
||||||
|
# for more discussion.
|
||||||
|
m = Module()
|
||||||
|
connect(m,
|
||||||
|
p=NS(signature=Signature({"a": In(1), "b": In(1)}),
|
||||||
|
a=Signal(),
|
||||||
|
b=Signal()),
|
||||||
|
q=NS(signature=Signature({"a": Out(1), "b": In(1)}),
|
||||||
|
a=Signal(),
|
||||||
|
b=Signal()))
|
||||||
|
|
||||||
|
|
||||||
class ComponentTestCase(unittest.TestCase):
|
class ComponentTestCase(unittest.TestCase):
|
||||||
def test_basic(self):
|
def test_basic(self):
|
||||||
|
|
Loading…
Reference in a new issue