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

all: only allow defining == and < and auto generate !=, >, >= and <= #8520

Merged
merged 5 commits into from
Feb 3, 2021
Merged

all: only allow defining == and < and auto generate !=, >, >= and <= #8520

merged 5 commits into from
Feb 3, 2021

Conversation

Delta456
Copy link
Member

@Delta456 Delta456 commented Feb 2, 2021

No description provided.

@Delta456
Copy link
Member Author

Delta456 commented Feb 2, 2021

The CI Error is not related to my PR.

@SleepyRoy
Copy link
Member

Good work! But wasn't the decision to only allow defining == and < ?

@JalonSolov
Copy link
Contributor

<= as well for performance.

@spytheman spytheman closed this Feb 2, 2021
@spytheman spytheman reopened this Feb 2, 2021
@SleepyRoy
Copy link
Member

I feel having <= for performance is not necessary.
We could achieve a <= b through !(b < a), which IMO doesn't have performance issue.

@Delta456
Copy link
Member Author

Delta456 commented Feb 3, 2021

I believe this implementation is fine as it is. We cannot restrict the user more than this. 🙂

@spaceface777
Copy link
Member

We cannot restrict the user more than this. 🙂

well, we can, if we disallow overloading one more operator 😉
If there is no performance benefit in overloading that additional operator, I don't see why it couldn't be disallowed as well.

@SleepyRoy
Copy link
Member

SleepyRoy commented Feb 3, 2021

As said hope it's the last time we touch this feature, so don't be that hurry.
I mean, with == and < defined,
a > b same as b < a
a >= b same as !(a < b)
a <= b same as !(b < a)
a != b same as !(a == b)
So I didn't see a performance issue here.
Allowing only 2 instead of 3 operator overload IMO better fits V's one-way philosophy.

@spytheman spytheman closed this Feb 3, 2021
@spytheman spytheman reopened this Feb 3, 2021
@Delta456 Delta456 changed the title all: only allow defining ==, < and <= and auto generate !=, > and <= all: only allow defining == and < and auto generate !=, >, >= and <= Feb 3, 2021
@medvednikov
Copy link
Member

@SleepyRoy a great summary.

@medvednikov medvednikov merged commit 7ec116d into vlang:master Feb 3, 2021
@medvednikov
Copy link
Member

Great job! A good simplification of the language.

@Delta456 Delta456 deleted the over_op_revamp branch February 3, 2021 14:19
@dumblob
Copy link
Contributor

dumblob commented Feb 4, 2021

I'm afraid we should definitely allow <= for performance reasons. The point is, that any non-trivial data types do often not have composable operations on the implementation level (they are composable only on semantical level). For such data types <= is not a composition of < and == because that'd be a waste of processing power. Imagine e.g. approximate comparisons of floats where == checks also for some "surrounding" and then < checks again for the same surrounding from one side.

Defining own methods for such composed operators is an easy way to get adequate performance.

@hungrybluedev
Copy link
Member

hungrybluedev commented Feb 4, 2021

@dumblob <= can be represented by negation of >. It solves the issue with == checking surrounding floats.

Also note that a > b == b < a. Flipping the operands is an important operation.

@dumblob
Copy link
Contributor

dumblob commented Feb 4, 2021

Yes, of course <= is a simple negation of > which in turn is the same as < but with flipped operands. Still, e.g. for floats there is a performance difference when implementing < and > "natively" (because floats are not linearly symmetric around a given number - i.e. close to zero they are much more precise than close to infinity). From that we can derive, that the programmer would like to choose the faster implementation and then negate that one (assuming double negation can't be easily optimized away because of function boundary).

Now the choice is made by V and not the programmer. So maybe allow defining either of < > instead of pre-selecting which of < > shall be implemented?

@SleepyRoy
Copy link
Member

@dumblob thanks for elaborating on it and I basically got your idea. I'm OK with your suggestion of allowing only one from < and >, depending on whose implementation is more favorable in a specific context.

@SleepyRoy
Copy link
Member

SleepyRoy commented Feb 4, 2021

But still it would be better to have some examples and let's analyze whether it's worth having this feature. I don't think one negation operation would be costly.

@Delta456
Copy link
Member Author

Delta456 commented Feb 4, 2021

@dumblob can you please join the discord server so that we can further discuss on this more?

@dumblob
Copy link
Contributor

dumblob commented Feb 4, 2021

I don't think one negation operation would be costly.

Well, for floats it will 😢.

@Delta456 I'd like to, but I won't be fast to respond interactively - I have hundreds of emails to read and thus I do things in batches. Feel free to discuss it there without me. Potentially we can do the "either of < >" thing after V 1.0 gets released as it sounds like 100% backwards compatible change/extension.

@Delta456
Copy link
Member Author

Delta456 commented Feb 9, 2021

@dumblob As per the discussion it has been stated that floating point comparison and equality are exact. It used to be approximate, but it has been reverted. Only able to use == and < keeps redundancy at a minimum while preserving the ability to write fast code and also auto-generating the other methods entails a negation at worst.

For approximate comparison and user-defined tolerance we will have math.Decimal which will solve your query hopefully.

@dumblob
Copy link
Contributor

dumblob commented Feb 9, 2021

@Delta456 this is a misunderstanding - please read carefully #5180 (comment) .

As per the discussion it has been stated that floating point comparison and equality are exact.

No, this is fully incorrect. Please accept this fact (as I begged already in the original thread).

There is nothing like exact comparison of inherently approximate representations (this is by definition and that's why I strongly suggest V not allowing any comparison between approximate data types by default without the programmer explicitly visibly choosing how to approach comparisons of approximate information).

Sounds theoretical? In practice you can test it yourself on your very own CPU - do millions of times the same C-style comparison in an (unoptimized) loop and at the same time do some other stuff on the same physical core (e.g. play a game or whatever). You'll see, that you'll get sometimes the comparison differently.

Please comment in the "float" thread and not here.

Only able to use == and < keeps redundancy at a minimum while preserving the ability to write fast code and also auto-generating the other methods entails a negation at worst.

I fully agree (as I said, to get rid of the additional negation we can enhance V some time after V 1.0 - if it proves valuable - by supporting something more as it'll be backwards compatible).

For approximate comparison and user-defined tolerance we will have math.Decimal which will solve your query hopefully.

Again, this is totally fine. What I'm complaining about is the default behavior which is currently undefined and thus wrong (both in theory and in practice).

So, introducing math.Decimal will not solve this problem. Having the comparison operators completely disabled by default for all approximate data (e.g. floats) would solve this problem (as the programmer would need to explicitly visibly select how to approach approximate comparisons - there are many options to choose from).

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.

10 participants