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:
- aspect-lreplace-fix.fbun [download] added by anonymous on 2015-02-19 20:47:02. [details]