Tcl Source Code

View Ticket
Login
Ticket UUID: 32b6159246574b9480f83800d4347126c3998af9
Title: lreplace where idx2 < idx1 not compiled correctly
Type: Bug Version: 8.6.3
Submitter: anonymous Created on: 2015-02-19 02:50:19
Subsystem: 47. Bytecode Compiler Assigned To: dgp
Priority: 8 Severity: Important
Status: Closed Last Modified: 2015-02-20 20:20:21
Resolution: Fixed Closed By: dgp
    Closed on: 2015-02-20 20:20:21
Description:
% lreplace {1 2 3} 2 0
1 2 2 3

It looks like TclCompileLreplaceCommand doesn't handle idx2 < idx1 properly.  Similar errors with >3 arguments too.

Hopefully the following patch formats correctly - chiselapp's struggling to sync.

It passes the extra tests, but negative indices might still break.

Of course, the case where numWords==4 and idx2<idx1 is the identity function, so it could be optimised further.


Index: generic/tclCompCmdsGR.c
==================================================================
--- generic/tclCompCmdsGR.c
+++ generic/tclCompCmdsGR.c
@@ -1498,10 +1498,14 @@

     tokenPtr = TokenAfter(tokenPtr);
     if (GetIndexFromToken(tokenPtr, &idx2) != TCL_OK) {
        return TCL_ERROR;
     }
+
+    if(idx2 != INDEX_END && idx2 < idx1) {
+       idx2 = idx1-1;
+    }
     
     /*
      * Work out what this [lreplace] is actually doing.
      */

==================================================================
--- tests/lreplace.test
+++ tests/lreplace.test
@@ -135,14 +135,23 @@
 } {}
 # Note that this test will fail in 8.5
 test lreplace-4.2 {Bug ccc2c2cc98: lreplace edge case} {
     lreplace { } 1 1
 } {}
+test lreplace-4.3 {lreplace edge case} {
+    lreplace {1 2 3} 2 0
+} {1 2 3}
+test lreplace-4.4 {lreplace edge case} {
+    lreplace {1 2 3 4 5} 3 1
+} {1 2 3 4 5}
+test lreplace-4.4 {lreplace edge case} {
+    lreplace {1 2 3 4 5} 3 0 _
+} {1 2 3 _ 4 5}
 
 # cleanup
 catch {unset foo}
 ::tcltest::cleanupTests
 return
 
 # Local Variables:
 # mode: tcl
 # End:
User Comments: dgp added on 2015-02-20 20:20:21:
Patch accepted for 8.6.4.

anonymous (claiming to be rkeene) added on 2015-02-19 20:47:38:
I attached a "fossil bundle" with this branch (minus the merge from trunk)

anonymous (claiming to be aspect) added on 2015-02-19 06:37:43:

Patch is now pushed to chiselapp: http://chiselapp.com/user/aspect/repository/tcl/timeline?r=aspect-lreplace-fix


anonymous (claiming to be aspect) added on 2015-02-19 02:55:27:
also a little suspicious of the block at line 1554:

	if (idx1 > 0 && idx2 > 0 && idx2 < idx1) {
	    idx2 = idx1 - 1;
	} else if (idx1 < 0 && idx2 < 0 && idx2 < idx1) {
	    idx2 = idx1 - 1;
	}

I'm not 100% clear, but it looks strange idx1 or idx2 == 0 isn't handled.

Attachments: