Jump to page: 1 2
Thread overview
Some problems with operator overloading
Mar 14, 2010
bearophile
Mar 15, 2010
Trass3r
Mar 15, 2010
bearophile
Mar 15, 2010
Trass3r
Mar 16, 2010
bearophile
Mar 16, 2010
Don
Mar 15, 2010
Philippe Sigaud
March 14, 2010
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
March 15, 2010
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
March 15, 2010
>> 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.
March 15, 2010
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
March 15, 2010
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
March 15, 2010
> 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!("==")
March 15, 2010
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


March 16, 2010
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
March 16, 2010
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
March 16, 2010
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
« First   ‹ Prev
1 2