RE: [jira] Commented: (STDCXX-901) 26.class.gslice test fails
Sorry for top posting, but this webmail app won't prefix existing lines. Argh! I've pulled the patch from stdcxx-901 as I've come up with a much more obvious and clean solution. I'll post an updated patch in the morning. Travis -Original Message- From: Travis Vitek [mailto:[EMAIL PROTECTED] Sent: Tue 6/3/2008 6:54 PM To: dev@stdcxx.apache.org Subject: RE: [jira] Commented: (STDCXX-901) 26.class.gslice test fails Okay, I have a fix. It isn't as pretty as I'd like it to be, but it is forward and backward binary compatible. I'm attaching the patch to the issue, which you can find by clicking the link below. Please review when you have a chance. http://issues.apache.org/jira/secure/attachment/12383346/stdcxx-901.patc h Note that I left some commented references to a new inline method gslice::is_empty(). If I submit the fix to 4.2.x I plan to remove these lines. When the fix goes to 4.3.x I'd like to use the clean solution which allows me to eliminate the friendship and direct member access. I also realize that the test is currently incomplete. I will obviously be working to finish up the test before submitting the patch. Travis
RE: [jira] Commented: (STDCXX-901) 26.class.gslice test fails
Okay, I have a fix. It isn't as pretty as I'd like it to be, but it is forward and backward binary compatible. I'm attaching the patch to the issue, which you can find by clicking the link below. Please review when you have a chance. http://issues.apache.org/jira/secure/attachment/12383346/stdcxx-901.patc h Note that I left some commented references to a new inline method gslice::is_empty(). If I submit the fix to 4.2.x I plan to remove these lines. When the fix goes to 4.3.x I'd like to use the clean solution which allows me to eliminate the friendship and direct member access. I also realize that the test is currently incomplete. I will obviously be working to finish up the test before submitting the patch. Travis
RE: [jira] Commented: (STDCXX-901) 26.class.gslice test fails
Martin Sebor wrote: > >Travis Vitek wrote: >> >> >>> Martin Sebor commented on STDCXX-901: >>> - >>> >>> This is from the binary incompatible rewrite of {{valarray}} >>> that I've been mentioning for eons (I should finally commit it >>> on trunk). If it fixes this bug it might spark an idea for a >>> compatible fix... >>> >> >> [...] >> >> int main () >> { >> // + length >> // |+- stride >> // || >> // vv >> test ("[EMAIL PROTECTED]", 0, "", ""); >> test ("[EMAIL PROTECTED]", 0, "0", "0"); >> test ("[EMAIL PROTECTED]", 0, "1", "1"); >> test ("[EMAIL PROTECTED]", 0, "1", "2"); >> test ("[EMAIL PROTECTED]", 0, "1", "3"); >> >> test ("[EMAIL PROTECTED]", 0, "2", "1"); >> test ("[EMAIL PROTECTED]", 0, "2", "2"); >> test ("[EMAIL PROTECTED]", 0, "3", "3"); >> test ("[EMAIL PROTECTED]", 0, "2", "3"); >> >> test ("[EMAIL PROTECTED]", 0, "5", "1"); >> test ("[EMAIL PROTECTED]", 1, "5", "1"); >> test ("[EMAIL PROTECTED]", 1, "5,2", "1,2"); >> >> test ("[EMAIL PROTECTED]", 0, "0,0,0", "1,2,3"); >> >> return 0; >> } >> >[...] >> The STLPort and RW implementations both get stuck in an >> infinite loop on >> the second testcase, so I'm pretty sure that they're broken. I can't >> find anything in the standard that says this would be unspecified or >> undefined behavior and I don't see anything that calls this out as a >> special case. This leads me to believe that this is a bug in both >> implementations and I should take it into consideration when applying >> any fix. > >I agree. There's more than one way to refer to the same slice. >Unless it's computationally expensive to avoid, none of them >should enter an infinite loop or any other kind of undefined >behavior. In that case, then I think your new gslice code [shown earlier in this thread] has issues with the same case. The following snip will calculate the wrong value for the number of indexes if an element of the length array is zero. This is the problematic code... for (size_t i = 0; i != length; ++i) { nelems *= _C_length [i]; } I believe the easy fix would be... for (size_t i = 0; i != length; ++i) { if (_C_length [i]) nelems *= _C_length [i]; } Then, of course you would need to fix the loop that calculates the values in the _C_inxs array to handle the special case while (inx && cache [inx - 1] == _C_length [inx - 1] - 1) --inx; The following should work. for (/**/; inx; --inx) { if ( _C_length [__n - 1] && cache [__n - 1] != _C_length [__n - 1] - 1) break; } Travis > >Martin >
Re: [jira] Commented: (STDCXX-901) 26.class.gslice test fails
Travis Vitek wrote: Martin Sebor commented on STDCXX-901: - This is from the binary incompatible rewrite of {{valarray}} that I've been mentioning for eons (I should finally commit it on trunk). If it fixes this bug it might spark an idea for a compatible fix... Martin, I'm struggling with what appears to a discrepancy between the standard and the two implementations I'm testing with. You mean a bug? ;-) According to lib.gslice.cons, a default constructed slice specifies no elements. Section lib.class.gslice describes gslices that do have lengths and strides. The example it gives is shown here [...] Assume for a moment that we have the following gslice... start = 0 length = { 0 } stride = { 0 } So the indices specified by this slice should be k = 0 + () * 0 Or just k = 0 because the sum is zero. i0, i1, k (0, (), 0) // = 0 + () * 0 So this slice should be a view of 0th element in a given array. That's my reading too. I wrote a quick testcase to make sure that I was understanding this gslice stuff correctly. It creates a valarray and a slice from strings, then assigns 0 to the resulting gslice_array. This zeros out all elements specified by the slice. Here is the set of testcases... int main () { // + length // |+- stride // || // vv test ("[EMAIL PROTECTED]", 0, "", ""); test ("[EMAIL PROTECTED]", 0, "0", "0"); test ("[EMAIL PROTECTED]", 0, "1", "1"); test ("[EMAIL PROTECTED]", 0, "1", "2"); test ("[EMAIL PROTECTED]", 0, "1", "3"); test ("[EMAIL PROTECTED]", 0, "2", "1"); test ("[EMAIL PROTECTED]", 0, "2", "2"); test ("[EMAIL PROTECTED]", 0, "3", "3"); test ("[EMAIL PROTECTED]", 0, "2", "3"); test ("[EMAIL PROTECTED]", 0, "5", "1"); test ("[EMAIL PROTECTED]", 1, "5", "1"); test ("[EMAIL PROTECTED]", 1, "5,2", "1,2"); test ("[EMAIL PROTECTED]", 0, "0,0,0", "1,2,3"); return 0; } [...] The STLPort and RW implementations both get stuck in an infinite loop on the second testcase, so I'm pretty sure that they're broken. I can't find anything in the standard that says this would be unspecified or undefined behavior and I don't see anything that calls this out as a special case. This leads me to believe that this is a bug in both implementations and I should take it into consideration when applying any fix. I agree. There's more than one way to refer to the same slice. Unless it's computationally expensive to avoid, none of them should enter an infinite loop or any other kind of undefined behavior. Martin
RE: [jira] Commented: (STDCXX-901) 26.class.gslice test fails
>Martin Sebor commented on STDCXX-901: >- > >This is from the binary incompatible rewrite of {{valarray}} >that I've been mentioning for eons (I should finally commit it >on trunk). If it fixes this bug it might spark an idea for a >compatible fix... > Martin, I'm struggling with what appears to a discrepancy between the standard and the two implementations I'm testing with. According to lib.gslice.cons, a default constructed slice specifies no elements. Section lib.class.gslice describes gslices that do have lengths and strides. The example it gives is shown here start = 3 length = { 2, 4, 3 } stride = { 19, 4, 1 } k = 3 + (0,1) * 19 + (0,1,2,3) * 4 + (0,1,2) * 1 i0, i1, i2, k (0, 0, 0, 3 ) // = 3 + (0) * 19 + (0) * 4 + (0) * 1 (0, 0, 1, 4 ) // = 3 + (0) * 19 + (0) * 4 + (1) * 1 (0, 0, 2, 5 ) // = 3 + (0) * 19 + (0) * 4 + (2) * 1 (0, 1, 0, 7 ) // = 3 + (0) * 19 + (1) * 4 + (0) * 1 (0, 1, 1, 8 ) // = 3 + (0) * 19 + (1) * 4 + (1) * 1 (0, 1, 2, 9 ) // = 3 + (0) * 19 + (1) * 4 + (2) * 1 (0, 2, 0, 11) // = 3 + (0) * 19 + (2) * 4 + (0) * 1 (0, 2, 1, 12) // = 3 + (0) * 19 + (2) * 4 + (1) * 1 (0, 2, 2, 13) // = 3 + (0) * 19 + (2) * 4 + (2) * 1 (0, 3, 0, 15) // = 3 + (0) * 19 + (3) * 4 + (0) * 1 (0, 3, 1, 16) // = 3 + (0) * 19 + (3) * 4 + (1) * 1 (0, 3, 2, 17) // = 3 + (0) * 19 + (3) * 4 + (2) * 1 (1, 0, 0, 22) // = 3 + (1) * 19 + (0) * 4 + (0) * 1 (1, 0, 1, 23) // = 3 + (1) * 19 + (0) * 4 + (1) * 1 ... (1, 3, 2, 36) // = 3 + (1) * 19 + (3) * 4 + (2) * 1 Assume for a moment that we have the following gslice... start = 0 length = { 0 } stride = { 0 } So the indices specified by this slice should be k = 0 + () * 0 i0, i1, k (0, (), 0) // = 0 + () * 0 So this slice should be a view of 0th element in a given array. I wrote a quick testcase to make sure that I was understanding this gslice stuff correctly. It creates a valarray and a slice from strings, then assigns 0 to the resulting gslice_array. This zeros out all elements specified by the slice. Here is the set of testcases... int main () { // + length // |+- stride // || // vv test ("[EMAIL PROTECTED]", 0, "", ""); test ("[EMAIL PROTECTED]", 0, "0", "0"); test ("[EMAIL PROTECTED]", 0, "1", "1"); test ("[EMAIL PROTECTED]", 0, "1", "2"); test ("[EMAIL PROTECTED]", 0, "1", "3"); test ("[EMAIL PROTECTED]", 0, "2", "1"); test ("[EMAIL PROTECTED]", 0, "2", "2"); test ("[EMAIL PROTECTED]", 0, "3", "3"); test ("[EMAIL PROTECTED]", 0, "2", "3"); test ("[EMAIL PROTECTED]", 0, "5", "1"); test ("[EMAIL PROTECTED]", 1, "5", "1"); test ("[EMAIL PROTECTED]", 1, "5,2", "1,2"); test ("[EMAIL PROTECTED]", 0, "0,0,0", "1,2,3"); return 0; } And here is the output I get with the Dinkum and gnu implementations... {1,1,1,1,1,1,1,1,1,1}[0,{},{}] = 0 => = {1,1,1,1,1,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{0},{0}] = 0 => {1,1,1,1,1,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{1},{1}] = 0 => {0,1,1,1,1,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{1},{2}] = 0 => {0,1,1,1,1,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{1},{3}] = 0 => {0,1,1,1,1,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{2},{1}] = 0 => {0,0,1,1,1,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{2},{2}] = 0 => {0,1,0,1,1,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{3},{3}] = 0 => {0,1,1,0,1,1,0,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{2},{3}] = 0 => {0,1,1,0,1,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{5},{1}] = 0 => {0,0,0,0,0,1,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[1,{5},{1}] = 0 => {1,0,0,0,0,0,1,1,1,1} {1,1,1,1,1,1,1,1,1,1}[1,{5,2},{1,2}] = 0 => {1,0,0,0,0,0,0,0,1,1} {1,1,1,1,1,1,1,1,1,1}[0,{0,0,0},{1,2,3}] = 0 => {1,1,1,1,1,1,1,1,1,1} The STLPort and RW implementations both get stuck in an infinite loop on the second testcase, so I'm pretty sure that they're broken. I can't find anything in the standard that says this would be unspecified or undefined behavior and I don't see anything that calls this out as a special case. This leads me to believe that this is a bug in both implementations and I should take it into consideration when applying any fix. Travis