clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.42k stars 150 forks source link

Clash can sometimes create duplicate port names #2528

Closed leonschoorl closed 1 year ago

leonschoorl commented 1 year ago
topEntity :: Bool -> Bool
topEntity x = x
{-# ANN topEntity
  ((defSyn "topEntity") { t_output = PortName "x" }
  ) #-}

generates

entity topEntity is
  port(x : in boolean;
       x : out boolean);
end;

This is a bit of a weird corner case, as far as I can tell you can only create it by partially specifying port names.

leonschoorl commented 1 year ago

It seems clash is smart enough to handle the reverse:

topEntity :: Bool -> Bool
topEntity result = result
{-# ANN topEntity
  ((defSyn "topEntity") { t_inputs = [PortName "result"] }
  ) #-}
entity topEntity is
  port(result   : in boolean;
       result_0 : out boolean);
end;
leonschoorl commented 1 year ago

Instead of trying to be smart about this, maybe we should simple check port names for uniqueness and report an error when they're not unique.

That'll also help users who accidentally use the same port name multiple times. Because right now clash happily accept:

topEntity :: Bool -> Bool
topEntity x = x
{-# ANN topEntity
  ((defSyn "topEntity") { t_inputs = [PortName "foo"], t_output = PortName "foo" }
  ) #-}

and generate invalid HDL

entity topEntity is
  port(foo : in boolean;
       foo : out boolean);
end;
martijnbastiaan commented 1 year ago

Duplicate of https://github.com/clash-lang/clash-compiler/issues/2246 ?

leonschoorl commented 1 year ago

Basically yes, with the addition of https://github.com/clash-lang/clash-compiler/issues/2528#issuecomment-1618561609