Tcl Source Code

In the TIP27-ification of tclIndexObj.c, the third
parameter of Tcl_GetIndexFromObjStruct() changed from a
'char **' to a 'CONST char **' (actually a CONST84 char
**).

In the actual use of the function though, the
'tablePtr' argument typically does not point (directly)
to a 'char *'  or a 'const char *', but rather to an
array of some user-defined structure type (whose first
member is a 'char *' or 'const char *'). 

So the most appropriate type is probably 'void
*structTablePtr' (which should be cast to a 'char *'
internally to do address arithmetic and to a 'CONST
char **' to extract the string value, just like it does
now.)

This change would also make it easier to write
extensions that compile against pre- and post-TIP27
versions of the core without resorting to #ifdefs or
-DUSE_NON_CONST; inserting a cast to (void *) would
suffice for pre-TIP27 versions.

Logged In: YES 
user_id=80530

committing on the blessing of the maintainer.  I've
fixed the documentation.  I kept the macro, though;
it's defined in the very same file, so availability
should not be a problem.

Logged In: YES 
user_id=61031

The attached patch only rises one question,regarding
generic/tclIndexObj.c line 77:
      STRING_AT(tablePtr,offset,0)
I think it is better simply to write:
      *(CONST char **) tablePtr
Probably those two are identical, but I am always reductant
to depend on the availability of such macros.

Apart from that there is a "*" too much in doc/GetIndex.3
line 39:
    .AP "CONST VOID" **structTablePtr in
this should be
    .AP "CONST VOID" *structTablePtr in

With those 2 little changes the patch is OK with me. Thanks
for the very fruitful discussion!

Regards,
              Jan Nijtmans

Logged In: YES 
user_id=80530

seems to work.  If it's good enough for the maintainer,
it's good enough for me.

File Added - 18224: patch

Logged In: YES 
user_id=68433

> Got a patch, anyone?

OK, see attached.  My changes to doc/GetIndex.3 should
probably be rewritten,
but it compiles cleanly and passes indexObj.test.

Logged In: YES 
user_id=80530

Got a patch, anyone?

Logged In: YES 
user_id=79902

Seems like a good plan to me, describing (as it does) that
it's a pointer to some unknown structure that won't get
modified...

Logged In: YES 
user_id=61031

One of the reasons for TIP 27 was to be able to use the Tcl
library in C++ code, even if input parameters were declared
const. So, what if we use "static const struct substOption"
in stead of "static struct substOption". The example code by
Jou still then works without any type-cast.

In that light, I would prefer to use (const void *) as the
second parameter of Tcl_GetIndexFromObjStruct, and that's
what it really is: a const pointer to an unknown structure.
Then the (SOME_CAST) in Jou's example code can simply be
left out, both in C and C++ code.

Is everybody happy with that alternative?

Logged In: YES 
user_id=68433

Hm, I'm having a hard time locating instances of
Tcl_GetIndexFromObjStruct in the wild.  As an example,
here's a modified version of the option-parsing part of
Tcl_SubstObjCommand; switching to
Tcl_GetIndexFromObjStruct() simplifies the code since we can
get rid of an enum declaration and a switch statement:

    static struct substOption {
        const char      *option;
        unsigned int    mask;
    } substOptions[] = {
        { "-nobackslashes", TCL_SUBST_BACKSLASHES },
        { "-nocommands",  TCL_SUBST_COMMANDS },
        { "-novariables", TCL_SUBST_VARIABLES },
        { NULL, 0 }
    };
    flags = TCL_SUBST_ALL;
    for (i = 1; i < (objc-1); i++) {
        if (Tcl_GetIndexFromObjStruct(interp, objv[i],
(SOME_CAST)substOptions,
                sizeof(substOptions[0]), "switch", 0,
&optionIndex) != TCL_OK) {
            return TCL_ERROR;
        }
        flags &= ~substOptions[optionIndex].mask;
    }

   /* ... */

With the old signature, "SOME_CAST" is 'char **'; with the
current signature 'SOME_CAST' is 'CONST84 char **'.  With
the proposed signature, a cast isn't necessary.

For cross-version compatibility, SOME_CAST can be 'void *',
which will work with all three signatures (except in C++
code, in which case it will only work if
Tcl_GetIndexFromObjStruct takes a 'void *').

But the main issue is that the "tablePtr" argument normally
points to a an array of structures of unknown type, so 'void
*' is the most appropriate type.

[later] 
> Payoff is really in calls to Tcl_GetIndexFromObj(), where 
you can let the compiler  auto-cast to (void *)

I don't think Tcl_GetIndexFromObj() should be changed, only
Tcl_GetIndexFromObjStruct().   The 'tablePtr' argument to
the former really _is_ a 'const char **'; declaring it as a
'void *' would only weaken the type-safety that TIP27 was
intended to enhance.

Logged In: YES 
user_id=80530

Maybe I'm getting it.  Payoff is really
in calls to Tcl_GetIndexFromObj(), where
you can let the compiler  auto-cast to (void *) ?

Got a patch we can try?

Logged In: YES 
user_id=80530

An example would help me understand.
Where is there extension code that calls
Tcl_GetIndexFromObjStruct(), and how would
a different signature help that code
compile against 8.3 and 8.4 headers?