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

std: lessThan and greaterThan between signed and unsigned #3001

Closed
wants to merge 1 commit into from

Conversation

shawnl
Copy link
Contributor

@shawnl shawnl commented Aug 3, 2019

It is a deviation from C, but I think we should consider making this the behavior
of the operators. See #2133

With vectors this behavior can be kinda expensive if the type sizes are 8, 16, 32, 64. (this does not implement vectors, but it is easy to do so with my other patch set). But premature optimization is the root of all evil, and I think its better to prefer readability of the code, and then catch minor performance stuff later.

This PR is partially to spark discussion, and demonstrate well-defined behavior.

std/math.zig Outdated Show resolved Hide resolved
@daurnimator daurnimator added the standard library This issue involves writing Zig code for the standard library. label Aug 4, 2019
@andrewrk andrewrk added this to the 0.5.0 milestone Aug 19, 2019
Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

I'm OK to merge this right now, but I want to propose an alternate API:

  • make Compare enum public and rename to CompareOperator
  • add EQ and NE to CompareOperator enum
  • pub fn compare(a: var, comptime op: CompareOperator, b: var) bool
  • delete lessThan, lessThanOrEqual, greaterThan, and greaterThanOrEqual

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 19, 2019
@andrewrk
Copy link
Member

I'll move this back to 0.5.0 if feedback is addressed

@andrewrk andrewrk removed this from the 0.6.0 milestone Oct 1, 2019
@shawnl shawnl force-pushed the lessThan branch 2 times, most recently from f381bc9 to ae8c98a Compare November 5, 2019 02:48
@data-man
Copy link
Contributor

data-man commented Nov 5, 2019

Maybe

pub const CompareOperator = enum {
    lessThan,
    lessThanOrEqual,
    equal,
    greaterThan,
    greaterThanOrEqual,
};

?

@shawnl
Copy link
Contributor Author

shawnl commented Nov 5, 2019

@data-man Yes, .equal is also useful, I didn't think of that.

@data-man
Copy link
Contributor

data-man commented Nov 5, 2019

Oh! I missed notEqual :-)

It is a deviation from C, but I think we should consider making this the behavior
of the operators. See ziglang#2133
@data-man
Copy link
Contributor

data-man commented Nov 6, 2019

@shawnl Please also add notEqual.
It's will be useful in many cases. E.g. DB ORMs, calculators, parsers, etc.
So developer will use this std enum instead of reinventing a wheel.

@shawnl
Copy link
Contributor Author

shawnl commented Nov 6, 2019

@data-man notEqual is just !compare(a, .equal, b)

@data-man
Copy link
Contributor

data-man commented Nov 6, 2019

Sure.
I propose new element in enum.

So developer will use this std enum instead of reinventing a wheel.

@andrewrk andrewrk closed this in 496f271 Dec 16, 2019
@shawnl
Copy link
Contributor Author

shawnl commented Dec 16, 2019

We could even support these comparisons between floats and integers, and floats of different bit widths. The only case I'm not sure about is when you have an integer that would round to a specific float (with a positive exponent) but is a different value, I think that should return not equal.

@andrewrk
Copy link
Member

Current plan for the language specification (#75) is to allow comparisons between floats & ints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants