clash-lang / clash-compiler

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

vecToTuple not needed anymore #2830

Open leonschoorl opened 1 week ago

leonschoorl commented 1 week ago
foo :: Vec 2 Bool -> Bool
foo xs = x && y
 where
   x :> y :> _ = xs

Starting with GHC 9.2 this produces warnings with -Wall enabled:

Blaat.hs:6:4: warning: [-Wincomplete-uni-patterns]
    Pattern match(es) are non-exhaustive
    In a pattern binding:
        Patterns of type ‘Vec 2 Bool’ not matched:
            Cons _ _
            :> _ (Cons _ _)
  |
6 |    x :> y :> _ = xs
  |    ^^^^^^^^^^^^^^^^

To workaround that we added vecToTuple in #2682:

foo :: Vec 2 Bool -> Bool
foo xs = x && y
 where
   (x, y) = vecToTuple xs

But I just discovered that since the addition of the COMPLETE pragma in #2716 the original code now compiles warning free.

And while vecToTuple was backported to the 1.8 branch, it hasn't made it into a released version yet.

I think we should revert the introduction of vecToTuple. (and backport #2716 to 1.8)

DigitalBrains1 commented 1 week ago

Is there a compelling reason to backport?

I'm inclined to say it's okay to backport, but I have this nagging feeling that since we ran into issues with Vec, its constructors and patterns before, that we might be overlooking some intricate aspect somewhere.

leonschoorl commented 1 week ago

In my mind the combination of reverting the addition of vecToTuple and the addition of the COMPLETE pragma went together, to keep the 1.8 functionally on par with its current state.

Not backporting the COMPLETE pragma would keep it on par with the current released 1.8 version.

But I don't have a strong preference either way.

DigitalBrains1 commented 1 week ago

I hadn't realised we backported vecToTuple. I must have accidentally skipped the part where you actually mention that above.

I'm fine with backporting the COMPLETE pragma.