www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Some problems with operator overloading

reply bearophile <bearophileHUGS lycos.com> writes:
In the following little programs can you help me tell apart my errors from bugs
in dmd?

(This post shows only part of the problems I have found with operator
overloading).

-------------------------------

1) Error in the documentation in this page:
http://www.digitalmars.com/d/2.0/operatoroverloading.html

This:

struct S {
    int opEquals(ref const S s) { ... }
}

Probably has to become like:

struct S {
    const bool opEquals(ref const(S) s) { ... }
}

-------------------------------

2) This code runs:


import std.c.stdio: printf;
struct Foo {
    int data;
    int opEquals(T:Foo)(T other) {
        printf("A");
        return this.data == other.data;
    }
}
void main() {
    int r = Foo(5) == Foo(5);
}


But I think dmd has to raise a c error, and require something like:
const bool opEquals(T:Foo)(ref const(Foo) other) {
Or:
const bool opEquals(T:Foo)(const(Foo) other) {
etc.

-------------------------------

3) This code runs:


struct Foo {
    int data;
    bool opBinary(string Op:"==")(ref const Foo other) { // wrong: opEquals
        return this.data == other.data;
    }
    bool opBinary(string Op)(ref const Foo other) if (Op == "0") { // wrong
        return this.data == other.data;
    }
}
void main() {}


But that can cause bugs: it's easy for programmers to forget to use opEquals
instead of opBinary("==").

And generally I'd like dmd to raise an error when silly ops are defined, like
that "0". It's important to avoid many bugs in the operator overloading usage.

-------------------------------

4) This program doesn't compile:


struct Foo {
    int x;
    Foo opUnary(string op:"++")() {
        this.x++;
        return this;
    }
}
void main() {
    Foo f = Foo(5);
    f++; // line 10
}


Prints:
temp.d(10): Error: var has no effect in expression (__tmp1)

-------------------------------

5) Here an implicit cast to uint or int can be handy, to avoid the cast (to be
able to create a generic "fake int" struct), is this possible?


struct Foo {
    int x;
    int opCast(T:int)() {
        return this.x;
    }
}
void main() {
    Foo f = Foo(5);
    auto a1 = new int[cast(int)f]; // OK
    auto a2 = new int[f];          // ERR
}


Philippe Sigaud in the digitalmars.D.learn newsgroup suggests an
opImplicitCast. It can be useful in other situations (but I don't know its
possible bad side effects):


struct Foo {
    int x;
    int opCast(T:int)() {
        return this.x;
    }
}
void bar(int i) {}
void main() {
    Foo f = Foo(5);
    bar(f); // Error
}

-------------------------------

Bye,
bearophile
Mar 14 2010
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Sun, 14 Mar 2010 10:55:41 -0400, bearophile <bearophileHUGS lycos.com>  
wrote:


 In the following little programs can you help me tell apart my errors  
 from bugs in dmd?


I'll preface my comments with a general comment.

operator functions themselves are not, and should not be, treated  
specially by the compiler.  You can write opBinary(int x) for all the  
compiler cares, and it's a valid function.  opX is not a keyword or  
specially treated symbol.

The way D works (and has always worked, even in D1) is that the operators  
themselves (e.g. '+' '-', etc.) are transformed into a function call.

The benefit to all this is that keyword pollution is kept to a minimum,  
and the compiler is extremely simplified.  Imagine if opBinary is treated  
as a keyword, you need a special cases littered throughout the code,  
special grammar, etc.  If opBinary is just another symbol, it's already  
covered by the compiler.  The only place it's special int the compiler is  
when it's *generating* opBinary, not when it's *parsing* opBinary.


 1) Error in the documentation in this page:
 http://www.digitalmars.com/d/2.0/operatoroverloading.html

 This:

 struct S {
     int opEquals(ref const S s) { ... }
 }

 Probably has to become like:

 struct S {
     const bool opEquals(ref const(S) s) { ... }
 }


I think there is no restriction on what opEquals in structs.  However,  
where opEquals is a virtual function, a const-correct signature must be  
required in order to override the base function.  I filed a recent bug on  
it, where in fact you can change an object through a const reference  
because the compiler forces it.


 -------------------------------

 2) This code runs:


 import std.c.stdio: printf;
 struct Foo {
     int data;
     int opEquals(T:Foo)(T other) {
         printf("A");
         return this.data == other.data;
     }
 }
 void main() {
     int r = Foo(5) == Foo(5);
 }


 But I think dmd has to raise a c error, and require something like:
 const bool opEquals(T:Foo)(ref const(Foo) other) {
 Or:
 const bool opEquals(T:Foo)(const(Foo) other) {
 etc.


Again, opEquals is not a special function.  Comparing two const Foos will  
correctly result in a compiler error.


 -------------------------------

 3) This code runs:


 struct Foo {
     int data;
     bool opBinary(string Op:"==")(ref const Foo other) { // wrong:  
 opEquals
         return this.data == other.data;
     }
     bool opBinary(string Op)(ref const Foo other) if (Op == "0") { //  
 wrong
         return this.data == other.data;
     }
 }
 void main() {}


 But that can cause bugs: it's easy for programmers to forget to use  
 opEquals instead of opBinary("==").


Again, opBinary and opEquals are simply functions.  There is nothing  
special about their existence, just about how the compiler rewrites the  
operators into calling those functions.  The fact that opBinary does not  
get called is the same as defining a function that does not get called.   
These kinds of errors are appropriate for a lint tool.


 And generally I'd like dmd to raise an error when silly ops are defined,  
 like that "0". It's important to avoid many bugs in the operator  
 overloading usage.


Note that opBinary can be directly called via:

myfoo.opBinary!("0")(other);


 -------------------------------

 4) This program doesn't compile:


 struct Foo {
     int x;
     Foo opUnary(string op:"++")() {
         this.x++;
         return this;
     }
 }
 void main() {
     Foo f = Foo(5);
     f++; // line 10
 }


 Prints:
 temp.d(10): Error: var has no effect in expression (__tmp1)


This is a bug, I think introduced by a recent change that makes f++ an  
lvalue.  Please file a bugzilla report.


 -------------------------------

 5) Here an implicit cast to uint or int can be handy, to avoid the cast  
 (to be able to create a generic "fake int" struct), is this possible?


 struct Foo {
     int x;
     int opCast(T:int)() {
         return this.x;
     }
 }
 void main() {
     Foo f = Foo(5);
     auto a1 = new int[cast(int)f]; // OK
     auto a2 = new int[f];          // ERR
 }


 Philippe Sigaud in the digitalmars.D.learn newsgroup suggests an  
 opImplicitCast.


opImplicitCast was abandoned for the more versatile alias this.  Something  
like this should work:

struct Foo {
   int x;
   int opCast(T:int)() { return this.x; };
   alias opCast this;
}

-Steve
Mar 15 2010
next sibling parent Trass3r <un known.com> writes:
 But that can cause bugs: it's easy for programmers to forget to use  
 opEquals instead of opBinary("==").


 Again, opBinary and opEquals are simply functions.  There is nothing  
 special about their existence, just about how the compiler rewrites the  
 operators into calling those functions.  The fact that opBinary does not  
 get called is the same as defining a function that does not get called.   
 These kinds of errors are appropriate for a lint tool.


Seconded.
The language specification is clear about that. == is not to be used with  
opBinary.
Mar 15 2010
prev sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Steven Schveighoffer:


I think there is no restriction on what opEquals in structs.<


I think this is a hole in the specification that needs to be filled, because I
think you must be sure opEquals returns a bool.



These kinds of errors are appropriate for a lint tool.<


Have you seen any good lint tool for D?

Generally any compiler error can be moved to a lint tool, so it's a matter of
balance. In my opinion generally if some sanity test on the code is
computationally heavy or the errors it spots are uncommon, or if it's not
certain they are really errors, or if it's very uncommon, then it's possible or
better to move it to a lint. If the test is not hard/heavy to do and it can
catch a common sure mistake then it's better to put it into the compiler. D
compiler already performs several tests that were work for C lint tools. 

So I think the compiler can be allowed to catch some potentially common
operator overloading bugs like opBinary("==").



Note that opBinary can be directly called via: myfoo.opBinary!("0")(other);<


And that's insane code, right. Today programmers want a nanny compiler that
helps. The age of strong silent C compilers is dead or it's acceptable for
kernel devs only.



This is a bug, I think introduced by a recent change that makes f++ an lvalue. 
Please file a bugzilla report.<


OK.



opImplicitCast was abandoned for the more versatile alias this.<


I fear that "alias this" sometimes can be a little too much versatile for the
programmer own good :o)



 Something like this should work:
 struct Foo {
    int x;
    int opCast(T:int)() { return this.x; };
    alias opCast this;
 }


This little program:

struct Foo {
   int x;
   uint opCast(T:uint)() { return this.x; }
   int opCast(T:int)() { return this.x; }
   alias opCast this;
}
void main() {
    Foo f = Foo(5);
    int[] a = new int[f]; // line 9
}


Gives the following errors:
test.d(9): Error: cannot implicitly convert expression (f) of type Foo to uint
test.d(9): Error: cannot cast f.opCast(T : int)

--------------------

Trass3r:


Seconded. The language specification is clear about that. == is not to be used
with opBinary.<


The specifications can be clear about all things, but one of the purposes of
the compiler is to help catch programmer errors too. If you don't agree then
please remove all error messages and safeties from the compilers you use and
have fun programming.

Thank you for your answers and comments,
bearophile
Mar 15 2010
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Mon, 15 Mar 2010 10:46:01 -0400, bearophile <bearophileHUGS lycos.com>  
wrote:


 Steven Schveighoffer:


 I think there is no restriction on what opEquals in structs.<


 I think this is a hole in the specification that needs to be filled,  
 because I think you must be sure opEquals returns a bool.


Oh, I thought you meant that the const decorations need to be in there.  I  
still think normal compilation should take care of this.


 These kinds of errors are appropriate for a lint tool.<


 Have you seen any good lint tool for D?

 Generally any compiler error can be moved to a lint tool, so it's a  
 matter of balance. In my opinion generally if some sanity test on the  
 code is computationally heavy or the errors it spots are uncommon, or if  
 it's not certain they are really errors, or if it's very uncommon, then  
 it's possible or better to move it to a lint. If the test is not  
 hard/heavy to do and it can catch a common sure mistake then it's better  
 to put it into the compiler. D compiler already performs several tests  
 that were work for C lint tools.

 So I think the compiler can be allowed to catch some potentially common  
 operator overloading bugs like opBinary("==").


It is a good point.  It's one of the drawbacks of a "lowering" technique  
-- getting one little bit wrong is not flagged as an error.  However, we  
haven't used the system very much yet, let's wait and see how pervasive  
such errors are.  I can see one potential reason to use opBinary("==") and  
that is to collect binary operator code in one place.  The opEquals()  
method could forward to the opBinary("==") function.

One thing that can help is to create in phobos such template constraints  
such as isBinaryOperator(string op) to safeguard against such problems.   
It would be better if the compiler did this, but I'm not sure how it's  
going to play out.  This may become the evil descendant of if(...); but it  
also could turn out to be nothing.  Maybe some really creative programmer  
will come up with a good reason to have it.


 Note that opBinary can be directly called via:  
 myfoo.opBinary!("0")(other);<


 And that's insane code, right. Today programmers want a nanny compiler  
 that helps. The age of strong silent C compilers is dead or it's  
 acceptable for kernel devs only.


This is not likely to be a common problem.  I don't think this is a good  
reason to put in a compiler error.


 Something like this should work:
 struct Foo {
    int x;
    int opCast(T:int)() { return this.x; };
    alias opCast this;
 }


 This little program:

 struct Foo {
    int x;
    uint opCast(T:uint)() { return this.x; }
    int opCast(T:int)() { return this.x; }
    alias opCast this;
 }
 void main() {
     Foo f = Foo(5);
     int[] a = new int[f]; // line 9
 }


 Gives the following errors:
 test.d(9): Error: cannot implicitly convert expression (f) of type Foo  
 to uint
 test.d(9): Error: cannot cast f.opCast(T : int)


Yeah, I tried to get it to work too, but it doesn't.  Even something like  
this doesn't work:

struct Foo {
    uint x;
    uint castToUint() { return x; }
    alias castToUint this;
}

This should work.  The template code you wrote above might not work,  
because you can't overload functions on no arguments.  I'm unsure how this  
should work, but at least something like this should:

alias opCast!uint this;

A bug filing is in order I think.

-Steve
Mar 15 2010
next sibling parent reply Trass3r <un known.com> writes:
 I can see one potential reason to use opBinary("==") and that is to  
 collect binary operator code in one place.


Currently dmd checks that and turns == into opEquals, should never become  
opBinary!("==")
Mar 15 2010
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Mon, 15 Mar 2010 16:59:53 -0400, Trass3r <un known.com> wrote:


 I can see one potential reason to use opBinary("==") and that is to  
 collect binary operator code in one place.


 Currently dmd checks that and turns == into opEquals, should never  
 become opBinary!("==")


What I meant was, opEquals could forward to opBinary, but if it's  
disallowed by the compiler, then that's not possible.  Also, if you  
defined opBinary like this:

auto opBinary(string op)(const ref T rhs)
{
...
}

Then should the compiler disallow this?  opBinary!("==") will compile.  I  
just think it's overkill to restrict what functions are valid arbitrarily.

-Steve
Mar 16 2010
parent reply bearophile <bearophileHUGS lycos.com> writes:
Steven Schveighoffer:

Thank you for this post, that gives me a chance to talk about this topic.


What I meant was, opEquals could forward to opBinary, but if it's disallowed by
the compiler, then that's not possible.<
I just think it's overkill to restrict what functions are valid arbitrarily.<


In D I think == is a special case because for classes it does more complex
things that are special for this case.

Constraints that are useful to prevent wrong code, like for example the type
system, often forbid few cases of correct code. So when you design the
constraints you must find a balance point in the middle.

I've written two little (< 500 lines) programs using the new operator
overloading, and I have found that /(even using the online docs, it's easy to
write wrong code (and currently those operators have about 8-10 bugs, found by
other people and me). The templates can be badly written, you can use the wrong
names, the strings can be the wrong ones or even wrongly written, you can miss
necessary operators, or even you can miss a corner case (== is a binary op, but
it has its own method name, so it's a special case. The first time I have
indeed used opBinary("==")).

You have to understand that the new operators are quite more bug-prone compared
to the old ones, the operator strings introduce new possible ways to write
bugs, that were not possible with the older system (and even with the old
operator overloading system it was not that hard to put bugs in the code).

So I think that it's better to have a front-end that helps me catch many
possible operator overloading bugs. Otherwise you probably will see many bugs
in that kind of code :-)



 Also, if you defined opBinary like this:
 auto opBinary(string op)(const ref T rhs) {...}
 Then should the compiler disallow this?  opBinary!("==") will compile.  


I don't ask the compiler to become magic and avoid any possible bug. I want the
compiler to do what's possible to help me avoid bugs when I use the new
operator overloading.

To avoid the need of a magic compiler then some convention can be adopted, and
the compiler can raise the syntax error when the programmer doesn't follow the
convention. This puts constraints, but they are good ones. I surely do not want
full freedom when I add operators to a struct/class, I want a compiler that
allows me to do only sane things, even if forbids me to do few possible things.
I don't want to program Perl with a static type system :-) D wants to be a
safer language.

So some way has to be invented to avoid that kind of bugs, and if that's not
possible, then the design of the operator overloading has to be fixed.

Bye,
bearophile
Mar 16 2010
next sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Tue, 16 Mar 2010 14:03:37 -0400, bearophile <bearophileHUGS lycos.com>  
wrote:


 Steven Schveighoffer:

 Thank you for this post, that gives me a chance to talk about this topic.


 What I meant was, opEquals could forward to opBinary, but if it's  
 disallowed by the compiler, then that's not possible.<
 I just think it's overkill to restrict what functions are valid  
 arbitrarily.<


 In D I think == is a special case because for classes it does more  
 complex things that are special for this case.


Is this the reason?  I'm actually curious why opEquals cannot be  
assimilated into opBinary.


 Constraints that are useful to prevent wrong code, like for example the  
 type system, often forbid few cases of correct code. So when you design  
 the constraints you must find a balance point in the middle.

 I've written two little (< 500 lines) programs using the new operator  
 overloading, and I have found that /(even using the online docs, it's  
 easy to write wrong code (and currently those operators have about 8-10  
 bugs, found by other people and me). The templates can be badly written,  
 you can use the wrong names, the strings can be the wrong ones or even  
 wrongly written, you can miss necessary operators, or even you can miss  
 a corner case (== is a binary op, but it has its own method name, so  
 it's a special case. The first time I have indeed used opBinary("==")).


I've seen plenty of incorrect-name bugs from newbies with the old operator  
system.  I don't think the new system is any more bug prone, it will just  
take time to get used to (as did the old system).


 Also, if you defined opBinary like this:
 auto opBinary(string op)(const ref T rhs) {...}
 Then should the compiler disallow this?  opBinary!("==") will compile.


 I don't ask the compiler to become magic and avoid any possible bug. I  
 want the compiler to do what's possible to help me avoid bugs when I use  
 the new operator overloading.


Then what you are asking for does not help much.  Essentially, the best  
test for the compiler to run is trying to compile opBinary!("=="), and if  
it does throw an error.  If this is not how it is done, then it's some  
kind of half-ass check, which can leave lots of room to still mess up.  I  
can think of at least three different ways to write opBinary so  
opBinary!("==") compiles.

But I'd rather not have the compiler impose such restrictions.  It  
complicates the compiler, and gives opBinary a half-keyword status when it  
is simply a symbol right now.


 To avoid the need of a magic compiler then some convention can be  
 adopted, and the compiler can raise the syntax error when the programmer  
 doesn't follow the convention. This puts constraints, but they are good  
 ones. I surely do not want full freedom when I add operators to a  
 struct/class, I want a compiler that allows me to do only sane things,  
 even if forbids me to do few possible things. I don't want to program  
 Perl with a static type system :-) D wants to be a safer language.

 So some way has to be invented to avoid that kind of bugs, and if that's  
 not possible, then the design of the operator overloading has to be  
 fixed.


I disagree.  I think people will learn the right way and just start doing  
it that way.  It remains to be seen, but I think changing things now is  
too much of a knee-jerk reaction.  We've had a total of one release with  
the new operator system!

-Steve
Mar 16 2010
prev sibling parent Don <nospam nospam.com> writes:
bearophile wrote:

 Steven Schveighoffer:
 
 Thank you for this post, that gives me a chance to talk about this topic.
 

 What I meant was, opEquals could forward to opBinary, but if it's disallowed
by the compiler, then that's not possible.<
 I just think it's overkill to restrict what functions are valid arbitrarily.<


 
 In D I think == is a special case because for classes it does more complex
things that are special for this case.



 
 Constraints that are useful to prevent wrong code, like for example the type
system, often forbid few cases of correct code. So when you design the
constraints you must find a balance point in the middle.
 
 I've written two little (< 500 lines) programs using the new operator
overloading, and I have found that /(even using the online docs, it's easy to
write wrong code (and currently those operators have about 8-10 bugs, found by
other people and me). The templates can be badly written, you can use the wrong
names, the strings can be the wrong ones or even wrongly written, you can miss
necessary operators, or even you can miss a corner case (== is a binary op, but
it has its own method name, so it's a special case. The first time I have
indeed used opBinary("==")).
 
 You have to understand that the new operators are quite more bug-prone
compared to the old ones, the operator strings introduce new possible ways to
write bugs, that were not possible with the older system (and even with the old
operator overloading system it was not that hard to put bugs in the code).


The thing which needs to be emphasised, is:

opBinary() is just for numeric programmers.

opEquals() is for everyone.

Defining == for a user-defined type is an everyday feature. Defining / 
is a niche feature.

I think that once a person has realized that, they'll have no problems 
with knowing that they should not use opBinary.
Mar 16 2010
prev sibling parent Philippe Sigaud <philippe.sigaud gmail.com> writes:
On Mon, Mar 15, 2010 at 16:12, Steven Schveighoffer <schveiguy yahoo.com>wrote:

(On alias this)


 Yeah, I tried to get it to work too, but it doesn't.  Even something like
 this doesn't work:

 struct Foo {
   uint x;
   uint castToUint() { return x; }
   alias castToUint this;
 }

 This should work.  The template code you wrote above might not work,
 because you can't overload functions on no arguments.  I'm unsure how this
 should work, but at least something like this should:

 alias opCast!uint this;

 A bug filing is in order I think.


I never got 'alias this' to work. Regularly, someone posts some cool
possible code here, and I can't get it to compile :(
And IIRC, you can have only one alias this in an object, which somewhat
limits its usefulness.

Philippe
Mar 15 2010