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

Use (:>) in favor of Cons #2914

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use (:>) in favor of Cons #2914

wants to merge 1 commit into from

Conversation

kleinreact
Copy link
Member

@kleinreact kleinreact commented Mar 30, 2025

We currently offer the Cons constructor and the (:>) pattern synonym for vector construction. They slightly differ in their type signature and implementation, which can cause some code to work with one but not with the other and the other way around (cf. #2913).

While the pattern synonym was necessary in the past to work around some GHC issues, these original problems are not not present with modern GHCs any more. Hence, it is desirable to favor the direct usage of the data constructor over the pattern.

Note that there are some conditional cases, where the currently differing type signature of the pattern may lead to less need for type annotations or explicit pattern matching on the constructor. This can be considered a side effect of the particular setup of the signature, which comes at the price that some code, which would check by using the data constructor instead, won't check with the pattern on the other hand. However, just requiring a bit more of annotation is a reasonable invest, as it usually leads to cleaner code. Consider for example this test case, that needed to be fixed for this change, where the resulting code resolved to be even easier to read than before.

In particular, the PR introduces the following changes:

  • It fixes the type signature of the pattern synonym to align with the one of the data constructor.
  • It swaps the names of (:>) and Cons. The use of (:>) is more popular than Cons. Almost all our examples use it, which makes it to be favored in comparison to Cons.
  • After the name swap, the pattern is the only remaining definition using the name Cons.
  • The resulting Cons pattern synonym is marked as deprecated, which will lead to only a single operation in the future.

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files

@kleinreact kleinreact mentioned this pull request Mar 30, 2025
@kleinreact kleinreact marked this pull request as ready for review April 8, 2025 07:41
@kleinreact kleinreact changed the title (:>) pattern simplification Use (:>) in favor of Cons Apr 8, 2025
@martijnbastiaan
Copy link
Member

martijnbastiaan commented Apr 11, 2025

One of the reasons we couldn't remove the pattern synonym last time is that lazy pattern matches didn't work on GADTs. Is that still the case?

Edit: if it is still the case, we could add an entry to the changelog saying users could use ViewPatterns to get the old behavior back. E.g., they would rewrite:

f ~(a :> _)

to

f (lazyV -> (a :> _))

@DigitalBrains1
Copy link
Member

Will this be a squash merge? Could you squash all commits and give it a good commit message? GitHub doesn't allow us to review the merge itself for some unfathomable reason, so I like to see the final product during review.

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

Successfully merging this pull request may close these issues.

3 participants