| Ticket UUID: | bc076f4f0e751deb62073dfad40eb5608b3a40be | ||
| Title: | Regression failures in large data tests | ||
| Type: | Bug | Created on: | 2023-08-12 16:22:25 |
| Submitter: | apnadkarni | Assigned to: | apnadkarni |
| Subsystem: | - New Builtin Commands | Severity: | Important |
| Priority: | 5 Medium | Last modified: | 2023-08-14 12:46:23 |
| Status: | Closed | Closed by: | apnadkarni |
| Resolution: | Fixed | Closed on: | 2023-08-14 12:46:23 |
| Version: | 9.0 | ||
| Description: | ||||
The following tests that were previously passing are now failing when tested with [8deca7d58a]. (That is not to say that commit caused failures as bigdata tests are normally disabled.)
==== string-last-bigdata-2.uncompiled,compiled $comment FAILED
---- Result was:
{4294967300 -1 4294967290 -1} {4294967300 -1 4294967290 -1}
---- Result should have been (exact matching):
{4294967300 -1 4294967290 1} {4294967300 -1 4294967290 1}
==== string-range-bigdata-1.uncompiled,compiled $comment FAILED
---- Result was:
{6 7 5 {} 5 4 {} 9 {}} {6 7 5 {} 5 4 {} {} {}}
---- Result should have been (exact matching):
{6 7 5 {} 5 4 {} 9 {}} {6 7 5 {} 5 4 {} 9 {}}
==== string-range-bigdata-1.uncompiled,compiled FAILED
==== string-range-bigdata-2.uncompiled,compiled $comment FAILED
---- Result was:
aXaaaa Xaaaaa
---- Result should have been (exact matching):
aXaaaa aXaaaa
==== string-range-bigdata-2.uncompiled,compiled FAILED
==== string-replace-bigdata-1.uncompiled,compiled $comment FAILED
---- Result was:
{789012345 012345678 XYZ789012345 012345678XYZ} {789012345 {} XYZ789012345 012345678XYZ}
---- Result should have been (exact matching):
{789012345 012345678 XYZ789012345 012345678XYZ} {789012345 012345678 XYZ789012345 012345678XYZ}
==== string-replace-bigdata-1.uncompiled,compiled FAILED
==== lindex-bigdata-1.uncompiled,compiled $comment FAILED
---- Result was:
{6 7 5 {} 5 4 {} 9 {}} {6 7 5 {} 5 4 {} {} {}}
---- Result should have been (exact matching):
{6 7 5 {} 5 4 {} 9 {}} {6 7 5 {} 5 4 {} 9 {}}
==== lindex-bigdata-1.uncompiled,compiled FAILED
==== lrange-bigdata-1.uncompiled,compiled $comment FAILED
---- Result was:
{6 {6 7} 7 5 {} 5 4 {} 9 {8 9} {}} {6 {6 7} 7 5 {} 5 4 {} {} {} {}}
---- Result should have been (exact matching):
{6 {6 7} 7 5 {} 5 4 {} 9 {8 9} {}} {6 {6 7} 7 5 {} 5 4 {} 9 {8 9} {}}
==== lrange-bigdata-1.uncompiled,compiled FAILED
| ||||
| User Comments: | ||||
apnadkarni added on 2023-08-13 04:00:02:
The breakage occurred with [0b05b5c750df]. Jan, I had rewritten the index encoding/decoding functions to deal with large indices correctly. The index processing is quite complicated so I had commented the various cases extensively. All tests passed at the time. The above commit either reverted or almost completely rewrote these functions causing the regressions. Was this intentional or inadvertent as part of merging in the fix for Tk indices [bug-a9929f112a]? apnadkarni added on 2023-08-13 10:46:34:
Branch bug-bc076f4f0e reverts the broken commit while also fixing Tk bug-a9929f112a. There don't seem to be test cases for that bug but the original bug steps and Csaba's sample script now work as expected. apnadkarni added on 2023-08-13 15:52:56:
I believe both large indices and Tk indexing is now working in the branch module bug [13d3c0d764] which already present in the trunk and in my view unfixable for the reasons stated there.
I plan on merging back into trunk if no one has any comments.
apnadkarni added on 2023-08-13 15:54:19:
Previous comment should read
I believe both large indices and Tk indexing is now working in the branch modulo bug [13d3c0d764] which already present in the trunk and in my view unfixable for the reasons stated there.
I plan on merging back into trunk if no one has any comments.
jan.nijtmans added on 2023-08-13 17:06:47:
I'm having vacation now, so I'm not able to really have a look. I'm trying to keep 8.7 and 9.0 in sync as much as possible. Should some of your changes be backported? If all testcases work as expected, feel free to commit. If (after my vacation) that turns out not to be true, I can always file another bug-report. It looks like you are starting to understand the challenge in getting index-handling 'right'. Thanks! apnadkarni added on 2023-08-14 12:46:07:
Re. 8.7. even my original changes were not in 8.7 since it does not support 64-bit indices anyways and the mods were directly related to that. Since the changes are not trivial I do not believe it is worth backporting and risking stuff breaking for no benefit. And regarding index handling, believe me, having rewritten TclIndexEncode to properly support 64-bit indices I understood the complexities well enough to have it working for Tcl itself with clear semantics without any failure modes or corner cases. The fact that the Tk's needs (TIP 577 or otherwise) have now introduced these "special" cases into Tcl indexing and the consequential corner case bugs like the lset one I logged seems to me to be a case of the tail wagging the dog. I don't know Tk well enough to judge if its desired semantics could not have been implemented in Tk itself. But whatever..changes now committed to trunk in 55602e8331. | ||||