TIP 59 Implementation

Logged In: YES
user_id=75003

The following files contain a partial implementation of TIP
#59. The generic parts are mostly done, also the gathering
of information for Unix. Windows and Mac are not adressed
yet.

Logged In: YES
user_id=75003

Deviations from the specification:

* Tcl_RegisterConfig creates a namespace and a
command in it. Both operations are require an
interpreter to modify. Thus added an interp
argument to the function.

* The struct holding key and value is public.
Hence it has to have the "Tcl_" prefix. Added.

Logged In: YES
user_id=75003

Looking at the tcl specific information I notice that not
all keys need the system encoding from --with-encoding.
Only the paths (...prefix,...) do. Using a global encoding
token means that I have to write the boolean values ("0",
and "1") in all possible encodings. This is not good.

A possible solution creates another deviation from the
specification: Remove "valEncoding" from the interface
of "Tcl_RegisterConfig" and move it into the structure
("Tcl_Config"). Then the writer of an extension has to
determine which values are encoded and which are
ASCII/UTF-8.

TIP 59 patch, unified diff, Version 3

Logged In: YES
user_id=75003

Updating the patch and the tcl configuration itself.

Tcl specific data to embed, Version 3

tclConfig.c, generic embedding support, Version 2

Logged In: YES
user_id=75003

Fixed bug in generic support part (Reference counting).
Did gathering of configuration information for the three
windows build systems:

- configure/Makefile ok, tested
- makefile.vc/rules.vc ok, untested
- makefile.bc ok, untested

I do not have a Macintosh, so no way for me to implement
the changes for that platform.

Changes from specification: The default encoding on windows
was changed from "iso8859-1" to "cp1252".

This entry will now be routed to a number of maintainers
for a first round of reviews based upon platform. I ask all
maintainers to pay particular attention and thought to my
comment here about the encoding for values (use
indicriminately for all, or only for user-specified values
like paths ?)

(1) Don Porter - Unix
(2) David Graveraux - Windows
(3) Daniel Steffen - Mac
(4) Jim Ingham - MacOSX

As there is no Mac stuff I hope that Daniel and Jim have
the time to implement this.

Assigning to Don now ...

Logged In: NO

Could this be put in CVS on a branch? I would like it if
it could be.

Logged In: YES
user_id=75003

Can do.

Logged In: YES
user_id=75003

Committed the current state of the work into the branch
"tip-59-implementation".

Logged In: YES
user_id=80530

Missing documentation and tests, of course.

The Makefile.in definitions of INSTALL_PREFIX,
INSTALL_EXEC_PREFIX, RUNTIME_EXEC_PREFIX do
not appear to be bulletproof. Consider the effects
of configure options --libdir= or --includedir= or
a value of TCL_PACKAGE_PATH that consists of more
than one directory.

The TIP says the array of Tcl_Config values is
terminated with an empty key, but it appears that
a NULL character pointer is used instead. Clarify
that when docs are written.

I note that all values are required to have the
same encoding. I think that's OK, but it is a
limitation.

"optimized" is stored as a boolean flag, but an
integer describing various levels of optimization
may match the possibilities better.

If line wrapping at 72 columns is not part of the
Tcl Engineering Manual, it should be.

The Tcl_Config array is not copied by Tcl_RegisterConfig;
instead a pointer to it is kept, so it needs to be
documented that the caller of Tcl_RegisterConfig cannot
allow that value to go away. The Tcl_Config array ought
to be static.

When creating the "wrap" to be used as ClientData for
the [pkgconfig] command, it makes more sense to me to
allocate space for n (Tcl_Obj *)'s rather than n (char *)'s
even though it technically doesn't make any difference.

Tcl_Alloc and Tcl_Free should not be used. Use ckalloc
and ckfree instead so that they are included in any
memory debugging.

Probably should have a better panic message than
"This can't happen". At least include the name of
the procedure.

Otherwise the implementation (at least the Unix parts)
appear to do what's specified in TIP 59. As recognized
in that TIP, though, that's just one step toward what
we need.

Logged In: YES
user_id=75003

Any ideas on how to test this ?
Docu, yes.

INSTALL_PREFIX, ... Yes these are the one critical part I
wanted reviewed. I'll try libdir and includedir.

Empty key / NULL. Ok, somehwere in my brain these two are
equivalent. I will clarify.

Regarding same encoding for all values, see the third
comment I made (counted from below).

Optimized as integer value: Is there agreement between the
various compilers (especially cross-platform) about the
meaning of an optimization level ? I don't believe so.

72 characters/line: Will reformat code.

Doc non-copying of array, making it static: Will do.

Alloc of (char*): I missed this one. I had char*'s first
and then switched to Tcl_Obj*'s.

Tcl_Alloc/ckalloc: Note line 701ff, file "tclCkAlloc" where
Tcl_Alloc etc are defined in terms of the debugging
functions if MEM_DEBUG is set. And line 989ff in the same
file if MEM_DEBUG is not set. IOW, both ckalloc and
Tcl_Alloc do the same regarding memory debugging.

Better panic: Ok

Logged In: YES
user_id=80530

Regarding ckalloc vs. Tcl_Alloc, you are correct that
memory debuggin will still be done. The problem is that
you must use ckalloc() to get the right values of __LINE__
and __FILE__ into the debugging reports. If you use
Tcl_Alloc, they will be "unknown" and 0.

It's not very helpful to learn that you have a memory
error at line 0 of an "unknown" file.

Logged In: YES
user_id=80530

BTW, see Bug 497459 for a patch that improves
the documentation about ckalloc/ckfree.

Logged In: YES
user_id=75003

I see your point regarding Tcl_Alloc/ckalloc now. I will
change my code.

Logged In: YES
user_id=75003

Comitted changes which address the following topics
from the comments below:

@ Reformatted to 72 columns per line (mostly).
Some character strings (Panic messages) are
still longer.

@ Better panic messages.

@ Allocation of wrapper uses sizeof(Tcl_Obj*)
instead of sizeof (char*).

@ Terminate array with either "" or NULL.

@ Changed to use ckalloc/ckfree instead of
Tcl_Alloc/Tcl_Free.

Logged In: YES
user_id=75003

Added manpage for Tcl_RegisterConfig. This should address
the missing documentation and also spots which had to be
clarified.

Logged In: YES
user_id=7549

committed some minor changes to makefile.vc and rules.vc

Just because symbols are turned off, doesn't mean the
compiler is optimizing. I added a compiler test to make
sure it supports optimizing and sets the flag
appropriately. I feel it's a more accurate way to go. I
also added the pentium errata switches which supposedly
fixes the FDIV hardware bug. More compiler testing could
be added, now that I figured a good way to do it. I'm
thinking of adding 'edit and continue' support...

Logged In: YES
user_id=75003

The optimization changes are fine with me. For
unix/configure we have either optimization on (-O2) or
symbols. The win/configure code does it similarly. I
believe that we should have a look and add the check you
did for VC there too.

edit/continue support ?

Logged In: YES
user_id=7549

Being an over user of step-debugging <G>, I really like.
Basically, it's compiling with -Zi instead of -Z7. It's
rather tied into the IDE, though, and just building with -
Zi doesn't mean the IDE can do an in-place
rebuild/reload/remap without using a .dsp for the build
instructions. I'll need to look into it and see.

Logged In: YES
user_id=7549

The optimizing test is simple:

$(cc32) -nologo -Ox -c -Zs -TC -Fdtemp nul 2>&1 |
find "D4002"

If "D4002" is found in stderr, then the VC++ compiler
doesn't support optimization which can be the case for the
low cost VC++ packages like the standard and student ones.
I hear find.exe doesn't work correctly on Win9x, but can't
verify that here.

Logged In: YES
user_id=75003

Ok, I understand your edit/continue now :)
Regarding the check, 'find not working correctly' means:
Does not exist ? Crashes ? ... Actually not relevant:
win/configure assumes a cygwin environment, which means I
can use 'grep' instead. Thanks.