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

Stdlib should provide a common set of infix operators on numeric data types #5458

Closed
vicuna opened this issue Dec 30, 2011 · 2 comments
Closed

Comments

@vicuna
Copy link

vicuna commented Dec 30, 2011

Original bug ID: 5458
Reporter: meyer
Status: resolved (set by @gasche on 2012-02-14T14:33:13Z)
Resolution: suspended
Priority: low
Severity: feature
Version: 3.13.0+dev
Category: ~DO NOT USE (was: OCaml general)
Monitored by: @gasche @protz gerd

Bug description

In particular Float, Int64, Int32, Bigint modules etc. should provide a nested module, that could be opened locally or using new expression opens.

<<
let bar x y =
let open Float.Infix in
x + y

val bar : float -> float -> float

let foo x y =
Int64.Infix.(x+y)

val foo : Int64.t -> Int64.t -> Int64.t

@vicuna
Copy link
Author

vicuna commented Dec 30, 2011

Comment author: @gasche

We have been experimenting with something like that in Batteries. In my experience, it is not trivial to get the right interface for this. I would advise experimenting with an external library at first to see if you find something that actually works well at least for you. Committing a change to the stdlib (with the implied backward-compatibility burden) looks like a dangerous idea to me.

In Batteries, we began with dot-suffixed operators, as floats: (+.), (-.), etc. for all numeric types. It was still a bit too heavy so we changed it to non-prefixed operators as your suggest, but the result is a mixed blessing: in practice, integers operators are still very useful even in contexts where you wish to compute on floats, bigints etc. (because they do not denote only numerical values but also indices of all sorts), and having to write t.(Int.(i+1)) inside you Float.Infix quantification doesn't work so well.

We also tried to move them from Float.Infix to Float directly. The reason is that while open Float.Infix works well and is explicit, for the quick local-open syntax M.(...) you rather want to say Float.(x * x + 2.) than Float.Infix.(...). It also turned out to be a debatable change, because people have taken the habit of opening Float globally, and the infix operator pollution hurts. My personal opinion is that this is mostly a defect of the "global open" idiom, but still.

An other thing that I have learned is: do not shadow the polymorphic comparison operators. Ever. At some point people found it convenient to define the equality for bigints as (=) in Bigint.Infix. This is extremely confusing: OCaml programmers are used to use (=) for equality everywhere, and it took me like twenty minutes to figure the typer error out the first time I was hit by this. It is ok to provide infix notations for domain-specific equalities, but it should not be (=) (I suggest (=.)). Not that I like the polymorphic equality.

PS: my opinion on the bug is therefore that it is not (yet?) of stdlib's concern. If I was sure about this, I would resolve the bug as "suspended". I'm not, and other people may have very different opinion. In case you agreed with my analysis, though, I suggest you resolve it yourself.

@vicuna
Copy link
Author

vicuna commented Dec 30, 2011

Comment author: meyer

Hello Gabriel,

We have been experimenting with something like that in Batteries. In
my experience, it is not trivial to get the right interface for
this. I would advise experimenting with an external library at first
to see if you find something that actually works well at least for
you. Committing a change to the stdlib (with the implied
backward-compatibility burden) looks like a dangerous idea to me.

I wasn't advising to mock up quickly a patch and then see what will
happen, it's rather an issue for open discussion if we really need it
and how useful would be that. I wouldn't also advice to add them to the
global context (e.g. operators to Float directly). However, I can see
that the idea it needs some more thinking than I thought initially.

From my personal experience, I would like to see infix operators for
each numerical type, in the form of generic infix names, or even for
each one for different ones. I think from the perspective of stdlib, stdlib
we should always choose more general solutions and keep them simple, so
I would opt as having them as a single set of operators.

To back my opinion, recently I wrote a lot of code using Int64 as a base
type, and I was suffering that I can't "open" the "right" operators into
the context.

In Batteries, we began with dot-suffixed operators, as floats: (+.),
(-.), etc. for all numeric types. It was still a bit too heavy so we
changed it to non-prefixed operators as your suggest, but the result
is a mixed blessing: in practice, integers operators are still very
useful even in contexts where you wish to compute on floats, bigints
etc. (because they do not denote only numerical values but also
indices of all sorts), and having to write t.(Int.(i+1)) inside you
Float.Infix quantification doesn't work so well.

I notice the Batteries team introduced them at some point.

As for your example you can always rebind the operator with let if you
want to avoid the local module open. You can also convert explicitly
to the more general (in terms of precision) type.

We also tried to move them from Float.Infix to Float directly. The
reason is that while open Float.Infix works well and is explicit,
for the quick local-open syntax M.(...) you rather want to say
Float.(x * x + 2.) than Float.Infix.(...). It also turned out to be a
debatable change, because people have taken the habit of opening Float
globally, and the infix operator pollution hurts. My personal opinion
is that this is mostly a defect of the "global open" idiom, but still.

Yes, I can agree that whatever it should be in nested module or not is
debatable however again from my personal opinion it should be wrapped.

An other thing that I have learned is: do not shadow the polymorphic
comparison operators. Ever. At some point people found it convenient
to define the equality for bigints as (=) in Bigint.Infix. This is
extremely confusing: OCaml programmers are used to use (=) for
equality everywhere, and it took me like twenty minutes to figure the
typer error out the first time I was hit by this. It is ok to provide
infix notations for domain-specific equalities, but it should not be
(=) (I suggest (=.)). Not that I like the polymorphic equality.

PS: my opinion on the bug is therefore that it is not (yet?) of
stdlib's concern. If I was sure about this, I would resolve the bug as
"suspended". I'm not, and other people may have very different
opinion. In case you agreed with my analysis, though, I suggest you
resolve it yourself.

OK. Thanks. I think that the discussion is a bit in the spirit of `syntax
and naming wars' :) , because there is no perfect solution for this, and the
proposed solutions are not significantly better than others.

The bottom line however is that, in most cases you just want to use
int32 or int63 or even bigint like you do with plain ints and you are not
able to do it right now in an easy way just by opening a module. So
I think we should provide a little bit of that boilerplate in some form.

However it's again a bit debatable and people might or not agree.

Thank you for your suggestions, I appreciate that you shared your
experience from the Batteries developers point of view and highlighted
some non trivial issues with this approach.

Cheers,
Wojciech

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

1 participant