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

[ cleanup ] Make Nat's NonZero to be an alias for IsSucc #3405

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

Conversation

buzden
Copy link
Contributor

@buzden buzden commented Oct 29, 2024

Description

Historically for some strange reason in Data.Nat we have two equivalent definitions: IsSucc and NonZero. Other standard types have standard pattern of Is<Constructor> type, ItIs<Constructor> constructor of proof and isIt<Constructor> for Dec Is<Consturctor>. IsSucc follows this pattern, when NonZero does not. However, NonZero is a handy name for this.

But the fact that there are two different types for this and the fact that some standard and non-standard functions use one of them, and others use the other, triggers a lot of people (I personally know around five of them).

What I suggest is to make NonZero to be an alias for IsSucc and to declare a deprecated alias for original constructor of NonZero to be backward compatible at least on usages at RHS.

Should this change go in the CHANGELOG?

  • If this is a fix, user-facing change, a compiler change, or a new paper
    implementation, I have updated CHANGELOG_NEXT.md (and potentially also
    CONTRIBUTORS.md).

@andrevidela
Copy link
Collaborator

If we can fix frex, this looks good to me

@buzden
Copy link
Contributor Author

buzden commented Oct 30, 2024

If we can fix frex, this looks good to me

Right, this seems to be not much a strech for frex, I made a PR there

Copy link
Collaborator

@mattpolzin mattpolzin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me too. Seems like we can get this merged whenever.

libs/base/Data/Nat.idr Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants