Tcl Source Code

Here's a first draft implementation
of TIP 157 as a patch to 
the HEAD.

Note changes to the public
interface.  Besides the
script level {expand} expansion,
a new Tcl_Token type

#define TCL_TOKEN_EXPAND_WORD 256

is in tcl.h and a new bytecode opcode

#define INST_LCONCAT1 100

is in tclCompile.h

Patch does not yet include documentation
or test updates.  Contributions for both
are welcome, as well as mucho banging
on it in search of bugs.

There are alternative implementation ideas
out there too, I'm sure, particularly on better
ways to handle bytecode compiling of the
{expand} operation.  Feel free to treat this
as the straw man.

Logged In: YES 
user_id=148712

Moved the discussion of the list-based alternative
(tip157.patch.2) to [Patch 842446]

File Added - 67522: 827119.patch

Logged In: YES 
user_id=80530

File 827119.patch is the
combined patch that is now
being applied to the HEAD.
Further optimization, additional
tests, etc. can take place as
ongoing development.

Logged In: YES 
user_id=79902

TIPs updated

File Deleted - 67067: 



File Added - 67480: tip157.patch.2

Logged In: YES 
user_id=32170

NB. In the TIP archive, tip144 needs to be withdrawn, and
tip157 needs to specify Tcl version 8.5 (not 9.0).

Logged In: YES 
user_id=148712

I agree with donal: patch approved, apply to HEAD. I'll let
you do it, as you mentioned a possible last round of changes.
We can still think of alternative implementations afterwards.

Logged In: YES 
user_id=79902

If the patch works, apply it.  If there are optimizations to
be done, they can be done later on.

File Added - 67441: tip157-imm-3.patch

Logged In: YES 
user_id=80530


here's another alternative
patch, tip157-imm-3.patch

It stores deltas between
words need expansion as
immediate bytes in the code
stream.  Also, it builds up
the delta list in a Tcl_DString
instead of a Tcl_ListObj for
less memory allocation overhead.
Also, unnecessary INST_LIST_VERIFY
opcodes are left out.

Could probably use more
whitebox tests tuned to
its particular branch points.

File Deleted - 67065: 



File Added - 67067: tip157.patch.2

Logged In: YES 
user_id=148712

oops - removing latest patch.3 (wrong), back to revised patch.2

File Deleted - 66997: 



File Added - 67065: tip157.patch.3

Logged In: YES 
user_id=148712

Updated patch.2 - slight change, added comments.

File Deleted - 66970: 



File Added - 66997: tip157.patch.2

Logged In: YES 
user_id=148712

Updated patch.2 (fix copy-paste error)

File Added - 66970: tip157.patch.2

Logged In: YES 
user_id=148712

Here's the 2nd revision of the "invoke list at stacktop"
approach. Improved efficiency in building the list, reducing
the memory churn.

File Added - 66826: tip157-imm-2.patch

Logged In: YES 
user_id=80530


Here's the next revision on the
index list as immediates in the
codestream approach.  The 
execution of the INST_INVOKE_EXP*
instructions are now simplified to not
attempt complex "in-place" expansion.

Instead expansion is done into the
stack expansion space above the
tosPtr.  Refcount management is
also left to the bytecode execution
engine macros.  This code should
be considerably simpler to understand.

File Added - 66775: tip157.patch.1

Logged In: YES 
user_id=148712

Enclosing a new patch tip157.patch.1

This is a modification of dgp's tip157.patch, keeping the
original logic of building the command to invoke as a list
object at stacktop. The differences to the original are:
   - new INST_INVOKE_LIST that replaces the call to
INST_EVAL_STK and fixes the object-safety bug
   - modification of INST_LCONCAT1 to improve efficiency.
The list handling in the bcc code can (and will) be improved
further; the hope is that the runtime penalty vs the
approach in tip157-imm.patch will be small enough to justify
using this simpler approach.

File Added - 66711: tip157-imm.patch

Logged In: YES 
user_id=80530


Here's an alternative patch that
emits lists of word indices to be
expanded directly into the
codestream (as new operand
types OPERAND_ULIST1
or OPERANCE_ULIST4).

New instructions
INST_INVOKE_EXP1 and
INST_INVOKE_EXP4 read
the immediate index lists and
perform expansion before 
going to a modified doInvocation
to take care of the command evaluation.

This patch corrects test basic-48.17.0,
indicating proper object safety of
expansion.

I'm keeping the former patch registered
this time, for sake of comparison by
interested reviewers.

File Deleted - 65603: 



File Added - 65724: tip157.patch

Logged In: YES 
user_id=80530


Latest patch defines and uses
a new opcode INST_LIST_VERIFY
to verify list validity before expansion
with less cost than before.

File Deleted - 65324: 



File Added - 65603: tip157.patch

Logged In: YES 
user_id=80530

New patch removes some
objv shifting in Tcl_EvalEx.

Logged In: YES 
user_id=80530

dkf's outline has many similarities
to what is done in the patch to
support argument expansion during
direct string evaluation.

Logged In: YES 
user_id=148712

Doc proposal exposed in the wiki:
http://mini.net/tcl/Dodekalogue

File Added - 65434: Tcl.n.patch

Logged In: YES 
user_id=148712

Attached my first attempt at Tcl.n revision for this Tip; RFC

Logged In: YES 
user_id=148712

Having trouble with the endekalogue: all my attempts to
document {expand} seem to require to incoporate the concept
of 'list' to Tcl.n, a concept which was not necessary up to now.
Does anybody have a simple proposal? I guess I deserve this ...

Logged In: YES 
user_id=79902

I suspect that we could use a better compilation strategy
than purely list concatenation, as that does a lot of
object-churn.  My idea goes something like this:
Two new opcodes (though one might be simulatable using
existing opcodes - I don't know for sure):
INST_VERIFY_LIST and INST_EXPAND_STK (might want it in 1/4
forms).
INST_VERIFY_LIST checks that the object on the top of the
stack is a list; it *doesn't* pop anything off the stack,
but it throws an error if it isn't a list at the stack top.
INST_EXPAND_STK has one immediate value which is the number
of words to evaluate (much as in INST_INVOKE_STK) but it
also reads an additional object off the top of the stack
(not counted?) which is a *sorted* list of integer indices
to expand.

Compilation proceeds as follows:
Generate words for command execution as normal.  If any of
the words are expanded, add a trailing INST_VERIFY_LIST
(required for correct functioning of basic-48.19.*) and
append the index of the word within the whole command
invokation to a list you are building.  If at the end of the
command you've got a non-empty list, push the list (which is
guaranteed to be sorted ints) and use INST_EXPAND_STK
instead of INST_INVOKE_STK. This simplifies the compiler
*and* allows for a more efficient strategy for use of
temporary space when building the actual list of argument
objects.

I proposed such a strategy for expansion compilation back in
the discussions for TIP#144 on tcl-core

File Deleted - 65227: 



File Added - 65324: tip157.patch

Logged In: YES 
user_id=80530


New patch updates the [eval]'s
found in the script library and
test suite that were only for
word expansion to use the
{expand} syntax, and those [eval]'s
that were used only to force direct
string evaluation to [testevalex].

No updates done to the bundled
pacakges, which may want to
consider the timing of when they
[package require Tcl 8.5].

Upon further reflection, it seems that
the failing test basic-48.17.0 really
is a bug, as performance/object safety
was one of the motivators for TIP 157.
Fixing it will require another bytecode,
and perhaps opens an opportunity for
the more ambitious bytecoders to look
for the best solutions.

File Deleted - 65203: 



File Added - 65227: tip157.patch

Logged In: YES 
user_id=80530

latest patch adds more tests
in compile.test, simplifies the
code in tclCompile.c a bit.

Note the knownBug tests that
highlight a potential ability to
overflow objc .

File Deleted - 65008: 



File Added - 65203: tip157.patch

Logged In: YES 
user_id=80530


Updated patch adds more tests,
moves those from cmdAH.test 
into basic.test, fixes a few bugs,
and add docs for the parser
changes.

Three tests fail, revealing not
exactly bugs, but inconsistencies
between compiled operation and
direct string evaluation.

File Deleted - 64974: 



File Added - 65008: tip157.patch

Logged In: YES 
user_id=80530

Updated patch adds several
tests to parse.test (and updates
it to tcltest 2)

File Deleted - 64922: 



File Added - 64974: tip157.patch

Logged In: YES 
user_id=80530


Updated patch corrects all
known flaws.  Includes
several tests from Peter
Spjuth.

Logged In: YES 
user_id=80530

Looks like an "off-by-one" in
the in-place objv array expansion
shift.  Will look again tomorrow.

Logged In: YES 
user_id=80530

Crash:

% set l1 [list a {b c} d]
a {b c} d
% list {expand}$l1
a {b c} d
% !!
list {expand}$l1
a {b c} d
% !!
list {expand}$l1
Segmentation fault

Note that the use of "!!" apparently
matters.

File Deleted - 64913: 



File Added - 64922: tip157.patch

Logged In: YES 
user_id=148712

The new INST was missing in the tclInstructionTable
(tclCompile.c); patch updated.

Logged In: YES 
user_id=80530


Probably worthwhile to note that
the controversial syntax choice
between TIPs 157 and 144 are
completely determined by line 337
of the patched tclParse.c.  Change

            static char expPfx[] = "expand";
to
            static char expPfx[] = "";

and this patch becomes an implementation
of TIP 144.

File Added - 64913: tip157.patch