Ticket UUID: | 2997642 | |||
Title: | many type casts needed when using Tcl_Pkg* API | |||
Type: | Patch | Version: | None | |
Submitter: | nijtmans | Created on: | 2010-05-06 14:37:11 | |
Subsystem: | 39. Package Manager | Assigned To: | nijtmans | |
Priority: | 9 Immediate | Severity: | ||
Status: | Closed | Last Modified: | 2010-09-09 04:14:37 | |
Resolution: | Fixed | Closed By: | nijtmans | |
Closed on: | 2010-09-08 21:14:37 | |||
Description: |
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. | |||
User Comments: |
nijtmans added on 2010-09-09 04:14:37:
allow_comments - 1 Done, so can be closed nijtmans added on 2010-09-01 04:02:35: 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. :-( jenglish added on 2010-08-31 01:35:03: 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.) nijtmans added on 2010-08-30 20:49:34: 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. nijtmans added on 2010-05-06 21:38:15: File Added - 373071: tcl-pkg.patch |
Attachments:
- tcl-pkg.patch [download] added by nijtmans on 2010-05-06 21:38:15. [details]