Tcl Source Code

View Ticket
Login
Ticket UUID: 2865247
Title: C version of [try] assumes return -level 0
Type: Bug Version: obsolete: 8.6b1
Submitter: a_kovalenko Created on: 2009-09-23 20:34:00
Subsystem: 18. Commands M-Z Assigned To: dgp
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2009-09-29 01:02:26
Resolution: Fixed Closed By: dgp
    Closed on: 2009-09-28 18:02:26
Description:
Try this on CVS head:
proc mumu {} { try { return -level 3 123} finally {} }
catch mumu rv ro
it gives 0 as result, -code 0 -level 0 as $ro
and on tcl 8.6b1 it gives (rightly)
2 as result, -code 0 -level 2 as $ro

The problem is caused by using TclProcessReturn in the C [try] implementation. TclProcessReturn expects -code and -level from the options to be the same as code and level passed as parameters. [try] implementation, on the other hand, passes constant 0 as level, and it may be wrong.

Proposed solution is to use Tcl_SetReturnOptions instead of TclProcessReturn in [try] implementation. I think it's the best of available alternatives, because it's symmetric to Tcl_GetReturnOptions call that is used to obtain these options in the first place.

Diff is attached.
User Comments: dgp added on 2009-09-29 01:02:26:

allow_comments - 1

dgp added on 2009-09-29 01:02:24:
        * generic/tclCmdMZ.c:   Replaced TclProcessReturn() calls with
        * tests/error.test:     Tcl_SetReturnOptions() calls as a simple fix
        for [Bug 2855247].  Thanks to Anton Kovalenko for the report and fix.
        Additional fixes for other failures demonstrated by new tests.

dgp added on 2009-09-28 23:35:43:
        * tests/error.test (error-15.9.*):      More coverage tests for [try].
        Test error-15.9.3.0.0 covers [Bug 2855247].

dgp added on 2009-09-28 10:23:16:
committed tests demonstrating related,
but different failures.

dgp added on 2009-09-25 02:02:53:
Since preservation of the level is not
provided for, and preservation of result
is questionable, and since in time we expect
most [try] evaluations to become bytecompiled,
doesn't seem worthwhile to make revisions
to permit more efficient, yet correct, calls
to TclProcessReturn().  Accepting the submitted
patch...

dgp added on 2009-09-25 02:01:16:
Appears to be another (potential?) error
here on line 4432:

    result = TCL_ERROR;

Since this comes between the time
result is captured as the return code
of the body evaluation, and the time
it is passed back to TclProcessReturn(),
it's at least an opportunity for the same
constraint to fail.

dgp added on 2009-09-25 01:25:20:
Good catch.

The comments before TclProcessReturn (hey, I
wrote them for once!) give the constraints:

"Note that the code argument must agree
with the -code entry in returnOpts and the
level argument must agree with the -level
entry in returnOpts,..."

Replacing with calls to Tcl_SetReturnOptions()
will enforce this constraint, so the submitted
patch is a correct one.  Before accepting, I'll
examine if there's a more direct way instead.

dkf added on 2009-09-24 04:58:12:
I've no time to look at this at a time when I'm alert enough. Assigning to someone who might know what to do (and who knows return options well...)

a_kovalenko added on 2009-09-24 03:34:00:

File Added - 344088: c-version-try.diff

Attachments: