SourceForge has been redesigned. Learn more.
Close

#3776 "wideInt" objType registration

obsolete: 8.5a7
closed
5
2007-09-19
2007-09-04
Don Porter
No

There's a developer tug-o-war
on the HEAD over whether to
register the "wideInt" Tcl_ObjType.

Filing this report to give us a
place to hash out the matter.

Initial assign to das to get his
attention.

Discussion

  • Daniel A. Steffen

    • assigned_to: das --> dgp
     
  • Daniel A. Steffen

    Logged In: YES
    user_id=90580
    Originator: NO

    Discussion on chat with kbk from earlier today about this matter, he is ok with the change, is there anything further that needs to be discussed ?

    <kbk> das?
    <das> yes?
    <kbk> I'm looking at your round of changes to tclObj.c - and feeling a bit uneasy.
    <das> the wideInt change ?
    <kbk> Yes...
    <kbk> ... You realize that Tcl_GetWideIntFromObj isn't guaranteed to leave a wideInt int rep?
    <das> yes
    <das> SetIntFromAny does not guarantee an intRep either AFAICT ?
    <kbk> ok... now, does whatever extension is calling Tcl_ConvertToType know that?
    <kbk> No... we've been moving steadily into a world where intRep's are the business only of the code that owns them.
    <das> as mentioned, I don't care about Tcl_ConvertToType() in ffidl
    <das> all I need is to be able to compare a given objects' type to wideInt
    <das> SetWideIntFromAny is unimportant to me, as mentioned it can be reverted
    <das> all I care about is the type registration
    <kbk> ok... now, why is it that ffidl needs to compare the type?
    <kbk> (Strikes me as a little dangerous to have runtime behaviour that changes with shimmering... but maybe I misunderstand what ffidl is doing)
    <das> passing 64bits through tcl unmolested, i.e. an obj created with a 64bit int by Tcl_NewWideIntObj by ffidl should be able to be converted back without going through bignums or floats, in a way compatible with both 8.4 and 8.5
    <das> if there was shimmering, we accept there may be lossiness, on 8.4 at least
    <das> on 8.5, I'm working on fixing that by going through bignums
    <kbk> Right. 8.5 should be lossless, though. Tcl_NewWideIntObj will create either an int or a wide (or a bignum only if NO_WIDE_TYPE is defined).
    <das> but I want to avoid the expense if there was no shimmering
    <kbk> Tcl_GetWideIntFromObj will retrieve exactly the wide that was stored.
    <kbk> Similarly, as long as you don't mess with tcl_precision, doubles are also lossless.
    <kbk> (except that signalling NaNs will be quieted, because nasal daemons lurk in that corner)
    <das> what ffidl does in 8.4 is use Tcl_GetWideIntFromObj only on objects of type wideInt...
    <das> otherwise use Tcl_GetDoubleFromObj
    <kbk> and presume that an int will fit in a double - which isn't true on a 64-bit machine.
    <das> right, the latter may be lossy e.g. if there was shimmering
    <das> this could be done differently on 8.5
    <das> but how to make an extension that works on both
    <das> with the same binary
    <das> in any case, existing ffidl binaries break due to the "wideInt" registration removal
    <kbk> Hmmm. You know the data type that you need?
    <das> I don't really see the harm in restoring it
    <kbk> Given that we've identified code that breaks, I'm inclined to agree... now I'm just thinking about what best to do moving forward.
    <das> it may well be that adding SetWideIntFromAny was a bad idea
    <kbk> SetFooFromAny in general is a bad idea. It's better to let the type be determined by Tcl_GetFooFromObj.
    <kbk> Do you know the data type that you need in a given context?
    <das> I'll have to check, it should be possible, but currently that's not the way the code works
    <das> mind you this is not code I wrote... it dates from 99...
    <kbk> Yeah. I know that ffidl has a bloody history... and numbers in Tcl are almost equally bad. (Bignums looked like the only way out because we repeated previous mistakes with unsigned wideInts)
    <kbk> (And the bignum library was going in anyway so that doubles could shimmer losslessly)
    <das> yes, and indeed ffidl has problems with both unsigned ints and unsigned wideints...
    <das> e.g. there is no way to distingush between -1, UINT_MAX and ULLONG_MAX and
    <das> at the tcl level...
    <das> looking to remedy that with the 8.5 rev, but staying compatible with 8.4 is tricky
    <kbk> Yeah. I *think* we managed to get things so that 8.4 puts numbers between INT_MAX+1 and UINT_MAX into wides. But unsigned wides are still a problem.
    <das> the problem is that ffidl forces such ints into an intrep, not a wideint (pre-8.4 code...)
    <kbk> *That* at least should be fixable.
    <das> yes
    <das> one issue is performance, often you just want quick roundtripping via tcl without any conversions, e.g. passing the return val of one ffidl callout as a param to another
    <das> I think that is the reason most of the type comparisons in ffidl are done currently
    <kbk> Again, 8.5 is getting reasonably smart about that. Most of the Tcl_GetFooFromObj where Foo is a numeric type recognize all the numeric types and don't shimmer.
    <kbk> (And yes, I understand that you need 8.4 compat... :( )
    <das> I''try and rip out all of that ugliness and see what happens... at least when running against 8.5
    <das> it would certainly simplify the code a lot...
    <kbk> If you find places where we get it wrong, by all means log it. "Shimmering" of numbers should now be 100% lossless (and in most cases won't shimmer)
    <das> of course I also wanted a minimal effort set of changes for 8.5...
    <das> great
    <kbk> In 8.6, I want to take out Tcl_ConvertToType, by the way. It serves no useful purpose.
    <das> sounds like a good idea, that's one thing ffidl is not using at least ;-)
    <das> so should I revert the SetWideIntFromAny addition ?
    <das> and leave just the type registration ?
    <kbk> Uhm. The way you've coded it, it's Mostly Harmless.
    <das> yes, intentionally ;-) the same as SetIntFromAny...
    <kbk> Oh, and don't register wideInt if NO_WIDE_INT_TYPE (or however it's spelt) is defined.
    <das> I don't think I do
    <das> the struct isn't even defined in that case
    <das> i.e. tclWideIntType
    <kbk> ok, cool. And otherwise, if we've identified a known dependency in an extension, then I think I'm ok with this. You might want to double-check with dgp as well.
    <das> ok
    <das> thanks

     
  • Daniel A. Steffen

    Logged In: YES
    user_id=90580
    Originator: NO

    and for more context, my email exchange with Joe English (prior to the chat above):

    From: Daniel A. Steffen
    Subject: Re: "wideInt" Tcl_ObjType
    Date: Tue, 4 Sep 2007 08:50:51 +1000
    To: Joe English
    Cc: kbk

    Hi Joe,

    On 04/09/2007, at 4:09, Joe English wrote:

    > Daniel --
    >
    > Re: recent change to generic/tclObj.c,
    >
    > | * generic/tclObj.c (TclInitObjSubsystem): restore registration of the
    > | "wideInt" Tcl_ObjType for compatibility with 8.4 extensions that access
    > | the tclWideIntType Tcl_ObjType; add setFromAnyProc for tclWideIntType.
    >
    >
    > Just confirmed with Kevin Kenny that this was removed
    > intentionally as part of the TIP#237 implementation
    > (generic/tclObj.c r1.96, 2005/10/08; see also
    > kennykb-numerics-branch).

    yes, I did realize that and looked into the CVS history as well, I committed the
    restoration fully aware of all this. I also reread briefly what the TIP says
    about Tcl_WideInt, but the reality in the code seemed to differ significantly
    with the stated desire to fully remove wide ints (which I chalked up to an
    obsolete TIP...)

    > Also, there is growing
    > consensus that neither the core nor extensions
    > should call Tcl_RegisterObjType() at all;
    > Tcl_ConvertToType() is broken as designed.

    I understand this, and I am in fact not using Tcl_ConvertToType() with the
    "wideInt" type; as far as I am concerned the new SetWideIntFromAny could be
    reverted, it was just added for completeness (and does not in fact convert to a
    wide int representation, same as SetIntFromAny).

    there is ample precedent in 8.5 for the restoration of Tcl_ObjType registrations
    that were removed and affected extensions: "boolean", "list" and "procbody" all
    had this happen, c.f. ChangeLog.2005, so I did not think much of reverting the
    "wideInt' registration removal when unintended consequences showed up.

    > Unless you know of a specific extension that relies on
    > the "wideInt" type being registered, it would be a good
    > idea to revert this change.

    the extension in question is ffidl, which I'm currently updating to support 8.5,
    my commits yesterday all have to do with this...

    ffidl needs to support passing 64bit numbers back and forth between tcl procs
    and user-specified C calls, it uses the "wideInt" Tcl_ObjType to compare against
    a given Tcl_Obj's type to avoid slow and potentially lossy conversions.

    existing ffidl sources (and binaries) broke when run against 8.5 because of the
    "wideInt" registration removal (arguably an error in ffidl, it did not expect
    Tcl_GetObjType to fail). I have fixed that bug, but the need to know (cheaply)
    if a given tcl object needs 64bit storage remains.

    Cheers,

    Daniel

    > Thanks,
    >
    >
    > --Joe English

     
  • Don Porter

    Don Porter - 2007-09-04

    Logged In: YES
    user_id=80530
    Originator: YES

    Thanks. Obviously I read my changelog
    messages before I read the chat log.

    The justifcation in "for the dumb guy"
    terms:

    ffidl contains Tcl_GetObjType() calls,
    but none of those calls check for the
    possibility of a NULL return value.
    Without that check, later testing against
    the saved return value can cause an
    "untyped" Tcl_Obj (objPtr->typePtr == NULL)
    to get treated as one of those types.

    There are ffidl binaries built from this
    source code out in the wild, and this bug
    will hit them unless the "wideInt" registration
    is restored to the HEAD.

    So, I accept the practical need to
    restore it, but I would very much like
    this bug in ffidl to be corrected so
    that when the next version binaries
    of ffidl make their way into the world,
    there's hope that someday we can follow
    through on our intent and remove this
    registration from Tcl.

    From the e-mail exchange:
    "...but the need to know (cheaply)
    if a given tcl object needs 64bit storage remains."

    I'd be interested in whether the
    internal routine TclGetNumberFromObj()
    would satisfy that need. That might be a
    good candidate to publicize for needs like
    this.

     
  • Don Porter

    Don Porter - 2007-09-04
    • assigned_to: dgp --> das
    • status: open --> pending
     
  • SourceForge Robot

    • status: pending --> closed
     
  • SourceForge Robot

    Logged In: YES
    user_id=1312539
    Originator: NO

    This Tracker item was closed automatically by the system. It was
    previously set to a Pending status, and the original submitter
    did not respond within 14 days (the time period specified by
    the administrator of this Tracker).