Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vecToTuple not needed anymore #2830

Open
leonschoorl opened this issue Oct 21, 2024 · 3 comments
Open

vecToTuple not needed anymore #2830

leonschoorl opened this issue Oct 21, 2024 · 3 comments

Comments

@leonschoorl
Copy link
Member

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
Copy link
Member

DigitalBrains1 commented Oct 21, 2024

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
Copy link
Member Author

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
Copy link
Member

DigitalBrains1 commented Oct 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants