www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - A opIndexUnary quiz

reply "bearophile" <bearophileHUGS lycos.com> writes:
A small quiz (it's a reprise of an older discussion). This code 
compiles with no errors, but do you see anything wrong here?


// -----------------
struct Foo {
     int x;
     alias this = x;
}

class Bar {
     Foo[] data;

     this() {
         data.length = 10;
     }

     Foo opIndex(uint i) {
         return data[i];
     }

     void opIndexUnary(string op)(uint i) if (op == "++") {
         data[i]++;
     }

     void opIndexUnaryRight(string op)(uint i) if (op == "++") {
         data[i]++;
     }
}

void main() {
     auto barfoo = new Bar;
     ++barfoo[3];
     assert(barfoo.data[3] == 1);
     barfoo[3]++;
     assert(barfoo.data[3] == 2);
}
// -----------------


Do you think this code should produce some compilation errors?

Bye,
bearophile
Jan 01 2013
next sibling parent =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 01/01/2013 09:52 AM, bearophile wrote:

 void opIndexUnaryRight(string op)(uint i) if (op == "++") {
 data[i]++;
 }


I admit that I could not see the error as I was reading the code. I 
thought that I was learning something new from that code. ;)


 Do you think this code should produce some compilation errors?


That is hard to answer. It makes me think that perhaps the special 
function names should all start with double underscores. Only then the 
compiler would have the right to produce a compilation error for the 
code above. :-/

Ali
Jan 01 2013
prev sibling next sibling parent reply "Era Scarecrow" <rtcvb32 yahoo.com> writes:
On Tuesday, 1 January 2013 at 17:52:20 UTC, bearophile wrote:

 A small quiz (it's a reprise of an older discussion). This code 
 compiles with no errors, but do you see anything wrong here?


  Well I see that you have opIndexUnary twice; According to the 
manual you wouldn't need as it would rewrite the code so you only 
need it once; Also they return void when they should return the 
value of x before/after the change.

  Also although the alias this makes the number increment, it 
should be specifying 'x', not that it seems to make a difference.


 Do you think this code should produce some compilation errors?


  As it is I don't see any problems...
Jan 01 2013
next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Tuesday, 1 January 2013 at 19:32:10 UTC, Era Scarecrow wrote:

 On Tuesday, 1 January 2013 at 17:52:20 UTC, bearophile wrote:

 A small quiz (it's a reprise of an older discussion). This 
 code compiles with no errors, but do you see anything wrong 
 here?


  Well I see that you have opIndexUnary twice; According to the 
 manual you wouldn't need as it would rewrite the code so you 
 only need it once; Also they return void when they should 
 return the value of x before/after the change.

  Also although the alias this makes the number increment, it 
 should be specifying 'x', not that it seems to make a 
 difference.


The notion of "right" does not really mean "on which side the 
operator is". Rather, "which operand does it apply to":

"A + B"

If A doesn't provide opBinary, then the compiler will move on to 
see if B provides opBinaryRight.

In the context of opUnary (and opIndexUnary), it makes no sense 
to talk about a "right" version. This should be caught (IMO) by 
the compiler.

Also, opUnary *can* return a void. However, in that case, the 
compiler will be unable to auto generate the post version of the 
unary operation. This is a legal design decision, taken (for 
example) by std.container.Array.
Jan 01 2013
prev sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Era Scarecrow:


  Well I see that you have opIndexUnary twice; According to the 
 manual you wouldn't need as it would rewrite the code so you 
 only need it once;


And as you have seen if you remove the useles opIndexRight the 
program keeps compiling with no errors and keeps asserting at 
run-time:



struct Foo {
     int x;
     alias this = x;
}

class Bar {
     Foo[] data;

     this() {
         data.length = 10;
     }

     Foo opIndex(uint i) {
         return data[i];
     }

     void opIndexUnary(string op)(uint i) if (op == "++") {
         data[i]++;
     }
}

void main() {
     auto barfoo = new Bar;
     ++barfoo[3];
     assert(barfoo.data[3] == 1);
     barfoo[3]++;
     assert(barfoo.data[3] == 2);
}


Bye,
bearophile
Jan 01 2013
next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
1/2/2013 7:52 AM, bearophile пишет:

 Era Scarecrow:


  Well I see that you have opIndexUnary twice; According to the manual
 you wouldn't need as it would rewrite the code so you only need it once;


 And as you have seen if you remove the useles opIndexRight the program
 keeps compiling with no errors and keeps asserting at run-time:



 struct Foo {
      int x;
      alias this = x;


Implicit conversion to int...


 }

 class Bar {
      Foo[] data;

      this() {
          data.length = 10;
      }

      Foo opIndex(uint i) {
          return data[i];
      }

      void opIndexUnary(string op)(uint i) if (op == "++") {
          data[i]++;


...and ++ somehow works with rvalue.

The fact that it's allowed is dangerous if you ask me.


      }
 }

 void main() {
      auto barfoo = new Bar;
      ++barfoo[3];
      assert(barfoo.data[3] == 1);
      barfoo[3]++;
      assert(barfoo.data[3] == 2);
 }


 Bye,
 bearophile



-- 
Dmitry Olshansky
Jan 02 2013
parent =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 01/02/2013 01:39 AM, Dmitry Olshansky wrote:

 1/2/2013 7:52 AM, bearophile пишет:



 struct Foo {
 int x;
 alias this = x;


 Implicit conversion to int...



 }



 class Bar {
      Foo[] data;



      this() {
          data.length = 10;
      }



      Foo opIndex(uint i) {
          return data[i];
      }



 void opIndexUnary(string op)(uint i) if (op == "++") {
 data[i]++;


 ...and ++ somehow works with rvalue.


I am not sure that it is an rvalue in this case. I think x is used 
directly as an lvalue. Since the post-increment operator cannot be 
overloaded in D, the compiler writes the previous expression as the 
equivalent of the following:

{
     int oldValue = data[i];
     ++data[i];
     oldValue;
}

Since data[i].x is an lvalue int, there shouldn't be a problem.

The problem that I have seen here is that although the programmer thinks 
that the post-increment operation is being defined by 
opIndexUnaryRight(), opIndexUnaryRight() is not a special name. We have 
seen a similar issue recently with opGet(). These names happen to look 
like operator overloads but they are not.

Ali
Jan 02 2013
prev sibling parent reply "Peter Summerland" <p.summerland gmail.com> writes:
On Wednesday, 2 January 2013 at 03:52:21 UTC, bearophile wrote:

 Era Scarecrow:


 Well I see that you have opIndexUnary twice; According to the 
 manual you wouldn't need as it would rewrite the code so you 
 only need it once;


 And as you have seen if you remove the useles opIndexRight the 
 program keeps compiling with no errors and keeps asserting at 
 run-time:



 struct Foo {
     int x;
     alias this = x;
 }

 class Bar {
     Foo[] data;

     this() {
         data.length = 10;
     }

     Foo opIndex(uint i) {
         return data[i];
     }

     void opIndexUnary(string op)(uint i) if (op == "++") {
         data[i]++;
     }
 }

 void main() {
     auto barfoo = new Bar;
     ++barfoo[3];
     assert(barfoo.data[3] == 1);
     barfoo[3]++;
     assert(barfoo.data[3] == 2);
 }


 Bye,
 bearophile


I am really just guessing, but it appears that whatever the post 
increment op gets rewritten as takes either a reference to or a 
copy of what is being incremented and it involves opIndex, not 
opIndexUnary as suggested by TDPL p. 378 and p. 369.

If opIndex is changed to return ref Foo, everything works. (I had 
change alias this = x to alias x this - 2.60 64 bit). Here is 
code that shows when opIndexUnary and opIndex is called and when 
the assertion fails for version(B) - with ref and version(A) 
without ref.

Note that TDPL recommends *not* having opIndex return a ref (p. 
378). Can we have out cake and eat it too?


import std.stdio, std.conv;

// -----------------
struct Foo {
   int x;
   alias x this;
   string toString() {
     return to!string(x);
   }
}

class Bar {
   Foo[] data;

   this() {
     data.length = 10;
   }

   version( A ) {
     Foo opIndex(uint i) {
       writefln("opIndex data[i]: %s",data[i]);
       return data[i];
     }
   }

   version( B ) {
     ref Foo opIndex(uint i) {
       writefln("opIndex data[i]: %s",data[i]);
       return data[i];
     }
   }

   Foo opIndexUnary(string op)(uint i) if (op == "++") {
     ++data[i];
     writefln("opIndexUnary data[i]: %s",data[i]);
     return data[i];
   }

}

void main() {
   auto barfoo = new Bar;
   assert(++barfoo[3] == 1);
   writeln("after ++barfoo[3]");
   assert(barfoo[3]++ == 1);
   writeln("after barfoo[3]++");
   assert(barfoo[3] == 2);             // Line 47 - fails for 
version(A)
   writeln("after barfoo[3]");
}
// -----------------


/*

$> dmd opndx.d -version=B
$> ./opndx
opIndexUnary data[i]: 1
after ++barfoo[3]
opIndex data[i]: 1
after barfoo[3]++
opIndex data[i]: 2
after barfoo[3]


$> dmd opndx.d -version=A
$> ./opndx
opIndexUnary data[i]: 1
after ++barfoo[3]
opIndex data[i]: 1
after barfoo[3]++
opIndex data[i]: 1
core.exception.AssertError opndx(47): Assertion failure

*/
Jan 02 2013
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 3 January 2013 at 00:08:12 UTC, Peter Summerland 
wrote:

 On Wednesday, 2 January 2013 at 03:52:21 UTC, bearophile wrote:

 Era Scarecrow:


 Well I see that you have opIndexUnary twice; According to the 
 manual you wouldn't need as it would rewrite the code so you 
 only need it once;


 And as you have seen if you remove the useles opIndexRight the 
 program keeps compiling with no errors and keeps asserting at 
 run-time:



 struct Foo {
    int x;
    alias this = x;
 }

 class Bar {
    Foo[] data;

    this() {
        data.length = 10;
    }

    Foo opIndex(uint i) {
        return data[i];
    }

    void opIndexUnary(string op)(uint i) if (op == "++") {
        data[i]++;
    }
 }

 void main() {
    auto barfoo = new Bar;
    ++barfoo[3];
    assert(barfoo.data[3] == 1);
    barfoo[3]++;
    assert(barfoo.data[3] == 2);
 }


 Bye,
 bearophile


 I am really just guessing, but it appears that whatever the 
 post increment op gets rewritten as takes either a reference to 
 or a copy of what is being incremented and it involves opIndex, 
 not opIndexUnary as suggested by TDPL p. 378 and p. 369.


That's a (old) bug. If opIndexUnary is present, then that should 
be called. However, currently, the compiler doesn't "see" it when 
it sees "a[i]++".

http://d.puremagic.com/issues/show_bug.cgi?id=5044

"++a[i]" works correctly (AFAIK). If you want to test opUnary, DO 
NOT toy around with the post increment index version. Just stick 
to simple unary:
"++a" and "a++"


 If opIndex is changed to return ref Foo, everything works. (I 
 had change alias this = x to alias x this - 2.60 64 bit). Here 
 is code that shows when opIndexUnary and opIndex is called and 
 when the assertion fails for version(B) - with ref and 
 version(A) without ref.


As soon as opIndex returns a ref, then *nothing* of 
opIndexSomething becomes necessary (heck, it becomes "counter 
necessary"). Same thing for front. If front returns by ref, then 
you don't need the "front(T t)" variant.

I don't have TDPL under my eyes right now, but I remember it 
explicitly stating that these functions where specifically meant 
to emulate access primitives, for functions that can't ref access 
to its primitives. In particular, for things like array bool.
Jan 02 2013
parent "Peter Summerland" <p.summerland gmail.com> writes:
On Thursday, 3 January 2013 at 07:03:23 UTC, monarch_dodra wrote:

 On Thursday, 3 January 2013 at 00:08:12 UTC, Peter Summerland 
 wrote:

 On Wednesday, 2 January 2013 at 03:52:21 UTC, bearophile wrote:

 Era Scarecrow:


 Well I see that you have opIndexUnary twice; According to 
 the manual you wouldn't need as it would rewrite the code so 
 you only need it once;


 And as you have seen if you remove the useles opIndexRight 
 the program keeps compiling with no errors and keeps 
 asserting at run-time:



 struct Foo {
   int x;
   alias this = x;
 }

 class Bar {
   Foo[] data;

   this() {
       data.length = 10;
   }

   Foo opIndex(uint i) {
       return data[i];
   }

   void opIndexUnary(string op)(uint i) if (op == "++") {
       data[i]++;
   }
 }

 void main() {
   auto barfoo = new Bar;
   ++barfoo[3];
   assert(barfoo.data[3] == 1);
   barfoo[3]++;
   assert(barfoo.data[3] == 2);
 }


 Bye,
 bearophile


 I am really just guessing, but it appears that whatever the 
 post increment op gets rewritten as takes either a reference 
 to or a copy of what is being incremented and it involves 
 opIndex, not opIndexUnary as suggested by TDPL p. 378 and p. 
 369.




Thanks for responding.

On rereading, maybe TDPL is just suggesting that the rewrite for 
a[i]++ should be handled exactly the same as foo++, where foo and 
a[i] are Foos. I.e., if the result of a[i]++ is not used, rewrite 
it as ++a[i]. That apparently is not happening, as reported in 
bug 5044. But if a[i]++ is used, then apply

(ref x){auto t=x; ++x; return t;} to (a[i])

which causes the function to be applied to a copy of a[i], 
resulting in the nop effect of a[i]++ (unless a[i] returns a ref, 
and we don't want that). So IMHO using the the general rewrite at 
p369 for indexed items is incorrect. And base don my probing, I 
am guessing that the general rewrite is indeed applied to a[i]++.

Would it be possible to have the compiler call 
opIndexUnary!"<op>"(b1, ...bk) where <op> could be, say, "post++" 
and "post--" in addition to "++" and "--", when appropriate? 
Seems like a simple solution.


 That's a (old) bug. If opIndexUnary is present, then that 
 should be called. However, currently, the compiler doesn't 
 "see" it when it sees "a[i]++".

 http://d.puremagic.com/issues/show_bug.cgi?id=5044

 "++a[i]" works correctly (AFAIK). If you want to test opUnary, 
 DO NOT toy around with the post increment index version. Just 
 stick to simple unary:
 "++a" and "a++"


Sorry for the messy example, but I did not mean to test opUnary. 
In the code, I just wanted to see if the rewrite of a[i]++ was 
calling it (which it is - in version A and version B).



 If opIndex is changed to return ref Foo, everything works. (I 
 had change alias this = x to alias x this - 2.60 64 bit). Here 
 is code that shows when opIndexUnary and opIndex is called and 
 when the assertion fails for version(B) - with ref and 
 version(A) without ref.


 As soon as opIndex returns a ref, then *nothing* of 
 opIndexSomething becomes necessary (heck, it becomes "counter 
 necessary"). Same thing for front. If front returns by ref, 
 then you don't need the "front(T t)" variant.

 I don't have TDPL under my eyes right now, but I remember it 
 explicitly stating that these functions where specifically 
 meant to emulate access primitives, for functions that can't 
 ref access to its primitives. In particular, for things like 
 array bool.
Jan 03 2013
prev sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Tuesday, 1 January 2013 at 17:52:20 UTC, bearophile wrote:

 A small quiz (it's a reprise of an older discussion). This code 
 compiles with no errors, but do you see anything wrong here?


 // -----------------
 struct Foo {
     int x;
     alias this = x;
 }

 class Bar {
     Foo[] data;

     this() {
         data.length = 10;
     }

     Foo opIndex(uint i) {
         return data[i];
     }

     void opIndexUnary(string op)(uint i) if (op == "++") {
         data[i]++;
     }

     void opIndexUnaryRight(string op)(uint i) if (op == "++") {
         data[i]++;
     }
 }

 void main() {
     auto barfoo = new Bar;
     ++barfoo[3];
     assert(barfoo.data[3] == 1);
     barfoo[3]++;
     assert(barfoo.data[3] == 2);
 }
 // -----------------


 Do you think this code should produce some compilation errors?

 Bye,
 bearophile


YES. Or should I say: There needs to be a way to diagnose these 
kinds of errors. I think there should be a "operator" keyword to 
protect yourself. I'd prefer it be mandatory, but even 
introducing it as optional would help:

//----
operator void opIndexUnaryRight(string op)(uint i);
//----
Error: No operator opIndexUnaryRight. Perhaps you meant 
opIndexUnary?
//----

This is especially improtant in that sometimes, the operator name 
is a guessing game...

==========================================
Related: I fixed a similar bug in std.container.DList recently: 
Can you spot the error?

//----
DList opOpassign(string op, Stuff)(Stuff rhs)  	
if (op == "~" && is(Stuff == DList))
//----

The fact that it made it into the standard library is, IMO, a 
tell tale sign of a big problem. A "smoking gun".
Jan 01 2013
parent "Era Scarecrow" <rtcvb32 yahoo.com> writes:
On Tuesday, 1 January 2013 at 21:19:04 UTC, monarch_dodra wrote:

 ==========================================
 Related: I fixed a similar bug in std.container.DList recently: 
 Can you spot the error?

 //----
 DList opOpassign(string op, Stuff)(Stuff rhs)  	
 if (op == "~" && is(Stuff == DList))
 //----

 The fact that it made it into the standard library is, IMO, a 
 tell tale sign of a big problem. A "smoking gun".


  I know it should be opOpAssign, but is easy to mess up...
Jan 01 2013