From: SourceForge.net <no...@so...> - 2010-09-08 17:13:32
|
Patches item #2997642, was opened at 2010-05-06 07:37 Message generated for change (Settings changed) made by jenglish You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=310894&aid=2997642&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 39. Package Manager Group: None Status: Open Resolution: Fixed Priority: 9 Private: No Submitted By: Jan Nijtmans (nijtmans) >Assigned to: Jan Nijtmans (nijtmans) Summary: many type casts needed when using Tcl_Pkg* API Initial Comment: Since Tcl 8.6, all stub pointers became "const". The Tcl_Pkg* functions have a ClientData parameter for registering them. Because this parameter is not const, it means that in tclBasic.c, tclTomMathInterface.c, tclStubLib.c, and tclTomMathStubLib.c type casts need to be used in order to silence the compiler warning. There must be a better way. Therefore, I suggest to change the API of the following 4 functions: Tcl_PkgProvideEx Tcl_PkgRequireEx Tcl_PkgPresentEx Tcl_PkgRequireProc so that they can be used without type casts in this situation. Attached patch shows how to do that. This is 100% source and binary compatible. The real changes are only in tcl.decls (and the generate tclDecls.h) and tclPkg.c. The other files don't need to be modified, but are just a demonstration how they can be cleaned up without type casts after the signature change. doc update is in the patch as well. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2010-08-31 14:02 Message: Remaining part committed. tclStubLib.c and tclTomMathStubLib.c change reverted as Twylite reported to me that MSVC++ 6.0 gives a warning with this construct. :-( ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2010-08-30 11:35 Message: Remembering that "ClientData" == "void *": Previous signature had: Tcl_PkgProvideEx(...., void *clientData); Tcl_PkgRequireEx(..., void **clientData_rtn); IOW: put pointer-to-T in, get void * back out, which the caller must convert back to a pointer-to-T. (Note that the PkgRequire() call did not require an explicit cast even under the old signature when using the usual idiom, and the PkgProvide call only needed a cast-away-from-const cast if the clientData is a pointer-to-const-T.) New signature has: Tcl_PkgProvideEx(...., const void *clientData); Tcl_PkgRequireEx(..., void *rtn); IOW: put a pointer-to-T or pointer-to-const-T in, get *anything* back out. This pokes an even bigger hole in the type system (including internally removing 'const' qualifiers), but since 'void *' is already involved that's probably not a big deal; we're already relying on the caller to get things right, the compiler won't help here. As long as it does not cause a regression of #1091431, it's probably OK. (Basically: make sure that if you're changing the usual idiom void *pkgData = NULL; if (Tcl_PkgRequireEx(...., &pkgData) != NULL && pkgData != NULL) fooStubsPtr = pkgData; to Tcl_PkgRequireEx(..., &fooStubsPtr); make sure that 'fooStubsPtr' doesn't get stomped in cases where the PkgRequire() fails. (It doesn't look like that should happen, but I can't really tell; the PkgRequire() codepath has gotten a lot more convoluted since last I looked at it.) ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2010-08-30 06:49 Message: Partly committed (except for Tcl_PkgProvideEx signature change). Joe, please evaluate. I am convinced this is a pure improvement, and 100% binary and source compatible, but if you can find any problem with it feel free to revert. The tclStubLib.c and tclTomMathStubLib.c modifications serve as demonstration how this change can benefit code. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=310894&aid=2997642&group_id=10894 |