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