Tcl Source Code

View Ticket
Login
Ticket UUID: 1917650
Title: Tcl_ConvertToType(, , doubleType) incompatibility
Type: Bug Version: obsolete: 8.5.2
Submitter: hkoba Created on: 2008-03-18 02:25:39
Subsystem: 10. Objects Assigned To: dgp
Priority: 9 Immediate Severity:
Status: Closed Last Modified: 2008-06-30 22:14:34
Resolution: Fixed Closed By: dgp
    Closed on: 2008-06-30 15:14:34
Description:
Dear TCT.

In 8.5.2rc1, I found Tcl_ConvertToType does not ensure 
conversion of obj to specific type. At least target type is double and string rep is like "100" (which fits to int), returned type is "int".

In other words, SetDoubleFromAny can return int in some case.

This can leads problems for C level extensions.
For example, I found problem in TclRAL. 
It uses Tcl_ConvertToType and then internalRep.doubleValue.

If this change is intentional, please change the manpage
of Tcl_ConvertToType.

Best regards.
User Comments: dgp added on 2008-06-30 22:14:34:
Logged In: YES 
user_id=80530
Originator: NO


and to HEAD.

dgp added on 2008-06-30 09:35:00:
Logged In: YES 
user_id=80530
Originator: NO


Doc updates committed for 8.5.3.

ferrieux added on 2008-06-17 15:36:21:
Logged In: YES 
user_id=496139
Originator: NO

> You went on to write a novel, but I'm getting
> off now.

Sorry, did not intend to spam you.

But had you mentioned TIP#249 earlier (thanks a million to Daniel for mentioning it), you would have saved me (and also Donal apparently) some time and headaches. Yes I know, the TIP collection is open, but I had no reason to suspect what was going on since:

 - TIP 249 is mentioned nowhere in comments in the code (contrarily to other TIPs)

 - Comments in the code are awfully misleading or just plain wrong

 - the pattern of Tcl_GetXFromObj calling (vs. being called by) SetXFromAny is still baffling, even when you know about the grand plan described in 249.

dgp added on 2008-06-17 11:43:15:
Logged In: YES 
user_id=80530
Originator: NO


This bug report is correct.  The docs
are out of date.  We'll get it fixed.

The old rule was the SetFooFromAny()
routine would set the Tcl_ObjType to
"foo".  In this case, SetDoubleFromAny()
would shimmer to the "double" Tcl_ObjType
or fail.  This routine forms the guts
of a Tcl_ConvertToType() call, so that
call would also produce a "double"
Tcl_ObjType or fail.

The new rule is that SetFooFromAny() sets
the internal rep to whatever Tcl_ObjType
it wants, so that future GetFooFromObj()
calls are fast compared with conversion
from string.  Tcl_ConvertToType() no longer
guarantees to convert to the Tcl_ObjType
you pass in, and the new advice is that
you shouldn't be calling Tcl_ConvertToType
in the first place.  Let the owner of the
Tcl_ObjType use it (and have the freedom
to change it).

For the particular example SetDoubleFromAny(),
it can shimmer to any numeric Tcl_ObjType.
That is, it does exactly what TclParseNumber()
does, take a string and produce the Tcl_Obj
with the most appropriate internal numeric
rep to hold the numeric value.  That's why
SetDoubleFromAny() is implemented as a simple
call to TclParseNumber().

There is no TCL_PARSE_DOUBLE flag, so I don't
understand your particular question.  And I
don't grok dkf's comments either.  I wonder
if we're looking at the same HEAD.

You went on to write a novel, but I'm getting
off now.

das added on 2008-06-17 07:32:27:
Logged In: YES 
user_id=90580
Originator: NO

Alexandre,

this subject has a long history, AFAIK the behaviour changes w.r.t
numberic types was a completely intentional part of TIP #249:

> First, as mentioned above, C extensions will no longer have fine control
> over Tcl's built-in numeric types, because the types will not be
> registered and hence will be unavailable for use with Tcl_ConvertToType.
> This is actually a good thing, since it means that the rest of Tcl can
> assume that they are well-behaved, resulting in a considerable
> simplification. Most of the Tcl Core Team believes that
> Tcl_ConvertToType has no legitimate use in any case.
> Second, it will no longer be correct to assume that Tcl_Get*FromObj will
> leave an internal representation of precisely the requested type. It is,
> in any case, a highly questionable practice for callers to assume a
> specific internal representation (with the possible exception of
> Tcl_Set*Obj and Tcl_New*Obj). There will no doubt be a few extensions
> that run afoul of this change, but they can be fixed easily in such a
> way that they will continue to compile and run on earlier versions of
> Tcl.

the original intent of the TIP to have the numberic ObjTypes unregistered
was relaxed because of backwards compatibility problems with extensions
that did not expect a NULL result from Tcl_GetObjType() (for instance I
ran into this with Ffidl).

Most of this is documented in the ChangeLog, and was also discussed
several times on tcl-core I think.

Kevin might be the best person to ask for further details.

There is a definitely doc bug for Tcl_ConvertToType here but probably
nothing more...

ferrieux added on 2008-06-17 04:26:55:
Logged In: YES 
user_id=496139
Originator: NO

> An object-type's setFromAnyProc is not something that is meant to be
> directly called by external code.

That's peculiar ! Why would the function be stored in a vector (Tcl_ObjType), if it were not to be called polymorphically ?
The natural tool for this polymorphic call is Tcl_ConverToType, which is in the main stubs. Hard to get more public...

Now there's the contract. The documentation says that an attempt at conversion is made, but also says that the TCL_OK/TCL_ERROR result can be used as a boolean test of success. So it quite clearly means that

  result==TCL_OK => final type == requested type

Now, it turns out that this contract is respected for all non-numeric types, and violated for all numeric types !
Worse, if we concentrate on those numeric types (in tclObj.c) and try to identify the relationships between the setFromAnyProc and the Tcl_SetXXXFromObj procs, we can see a total mess:

TYPETcl_GetXXXFromObjSetXXXFromAny

intnot always shimmer(+parse_integer)convergent
widenot always shimmer(+parse_integer)<-
bignumnot always shimmer(+parse_integer)none
doublenot always shimmer (+parse any)->
booleannot always shimmer (+parse any)convergent


In this table, the third column means:
  convergent: both procs call a third one.
  none: the setupProc is NULL
  -> T_GXFO calls the SFAproc
  <- the SFAproc calls T_GXFO

The second column warns that the T_GXFO take care to avoid shimmering when possible
The (+parse...) tels which flags are passed to TclParseNumber.

Again, this is messy. But what about cleaning that up a bit ?
My proposal would be to keep the non-shimmering feature of T_GXFO (since they all have an output parameter distinct from the obj), while enforcing the type contract for the SFAproc, when non-NULL (bignums have a null SFAproc, just like Cons).

Also, why Tcl_Panic in Tcl_ConvertToType for types with a null SFAproc ?
Why not simply return TCL_ERROR ?

Reactions ?

-Alex

dkf added on 2008-06-15 21:54:51:
Logged In: YES 
user_id=79902
Originator: NO

An object-type's setFromAnyProc is not something that is meant to be directly called by external code.

But it's not clear to me why the one for the double type is trying to be so smart. The smartness should be in Tcl_GetDoubleFromObj or Tcl_GetNumberFromObj or something like that (and those should be the only functions that call SetDoubleFromObj, even indirectly...)

ferrieux added on 2008-06-15 05:03:44:
Logged In: YES 
user_id=496139
Originator: NO

Don, when you're back from vacation, be sure to email me on this. The one-line patch is tempting.

ferrieux added on 2008-06-01 17:32:25:
Logged In: YES 
user_id=496139
Originator: NO

Re the too weak TclParseNumber, specifically, why does the call in SetDoubleFromAny use a zero flag instead of TCL_PARSE_DOUBLE ?

ferrieux added on 2008-05-31 17:30:17:
Logged In: YES 
user_id=496139
Originator: NO

Don, while I agree about the recommended way, I'm wondering whether it is wise to have a setFromAny function fork in such a weak fashion. Indeed, SetDoubleFromAny just delegates to TclParseNumber which has a very broad spectrum...

In addition, I propose a slight style fix in Tcl_GetDoubleFromObj: a permutation of the two lines:

  *dblPtr = (double) objPtr->internalRep.doubleValue;
and
  *dblPtr = objPtr->internalRep.longValue;

which occur in the wrong cases (the (double) cast occurs on an already-double value, and the cast-less assignment effectively casts an long to a double...).

hkoba added on 2008-03-19 22:28:19:
Logged In: YES 
user_id=148473
Originator: YES

> The correct way...

Thank you for your advice. I will contact to the author of TclRAL.

dgp added on 2008-03-18 22:32:50:
Logged In: YES 
user_id=80530
Originator: NO


Thanks for the alert about the documentation.

The correct way to pull a double value
from a Tcl_Obj() is with the
Tcl_GetDoubleFromObj() routine.

hkoba added on 2008-03-18 09:25:40:

File Added - 270842: tcl-convertto-double.c

Attachments: