Tcl Source Code

View Ticket
Login
Ticket UUID: e711ffb4583d8183a57cba36ab197c18721f6049
Title: TclIsLocalScalar() appears wrongheaded
Type: Bug Version: all
Submitter: dgp Created on: 2014-11-26 20:08:15
Subsystem: - New Builtin Commands Assigned To: dgp
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2014-12-23 02:42:55
Resolution: Fixed Closed By: dgp
    Closed on: 2014-12-23 02:42:55
Description:
While looking at [d2ffcca163] there was briefly
a false lead that the trouble was caused by TclIsLocalScalar().
That was wrong, but prompted a look at what TILS() was doing
since it appeared to be doing nothing that made much sense.

All callers of TILS are in tclCompCmds.c -- routines that 
compile commands into bytecode.  All those routines operate
on Tcl_Token streams as input.  The parser has already had
its turn.  Every TILS call is on strings that are already
marked by the parser as being SIMPLE_WORD.  If they have
special characters, we already know at this point they've been
quoted so as to neuter their specialness.

What's wanted where TILS calls are made is a simpler task
of avoiding arrays and qualified variable names, and the
right tool for that appears to be PushVarName (still
confirming that).

Here's a demo of what's (harmlessly) wrong.  First a
base case for comparison:

% proc demo {} {catch {} {r}; puts '${r}'}
% tcl::unsupported::disassemble proc demo
ByteCode 0x0x1779d4b0, refCt 1, epoch 3, interp 0x0x17748250 (epoch 3)
  Source "catch {} {r}; puts '${r}'"
  Cmds 2, src 25, inst 36, litObjs 4, aux 0, stkDepth 4, code/src 0.00
  Proc 0x0x17774b40, refCt 1, args 0, compiled locals 1
      slot 0, scalar, "r"
  Exception ranges 1, depth 1:
      0: level 0, catch, pc 5-12, catch 11
  Commands 2:
      1: pc 0-22, src 0-11         2: pc 23-34, src 14-24
  Command 1: "catch {} {r}"
    (0) beginCatch4 0
    (5) push1 0         # ""
    (7) push1 1         # "0"
    (9) jump1 +4        # pc 13
    (11) pushResult
    (12) pushReturnCode
    (13) endCatch
    (14) reverse 2
    (19) storeScalar1 %v0       # var "r"
    (21) pop
    (22) pop
  Command 2: "puts '${r}'"
    (23) push1 2        # "puts"
    (25) push1 3        # "'"
    (27) loadScalar1 %v0        # var "r"
    (29) push1 3        # "'"
    (31) concat1 3
    (33) invokeStk1 2
    (35) done


And now nearly the same case with TILS() taking us down the
wrong path:

% proc demo {} {catch {} {[}; puts '${[}'}
% tcl::unsupported::disassemble proc demo
ByteCode 0x0x1779cbe0, refCt 1, epoch 3, interp 0x0x17748250 (epoch 3)
  Source "catch {} {[}; puts '${[}'"
  Cmds 2, src 25, inst 22, litObjs 5, aux 0, stkDepth 4, code/src 0.00
  Proc 0x0x17772930, refCt 1, args 0, compiled locals 1
      slot 0, scalar, "["
  Commands 2:
      1: pc 0-8, src 0-11          2: pc 9-20, src 14-24
  Command 1: "catch {} {[}"
    (0) push1 0         # "catch"
    (2) push1 1         # ""
    (4) push1 2         # "["
    (6) invokeStk1 3
    (8) pop
  Command 2: "puts '${[}'"
    (9) push1 3         # "puts"
    (11) push1 4        # "'"
    (13) loadScalar1 %v0        # var "["
    (15) push1 4        # "'"
    (17) concat1 3
    (19) invokeStk1 2
    (21) done

Whether the variable name is "r" or "[" makes
no difference, so we're falling back to command
invocation for no good reason.  TILS is testing
the wrong thing for the wrong reason and generating
less good (though correct) code as a result.
User Comments: dgp added on 2014-12-23 02:42:55:
and now on trunk too.

dgp added on 2014-12-19 16:09:28:
Corrected in 8.5 branch

dkf added on 2014-12-12 02:51:52:

Don't forget that the trunk may have additional uses, as the set of compiled things expanded after 8.5. Much of my use of such things as TILS in command compilers is by using existing functioning patterns, rather than analysing what things do in great depth.