Menu
▾
▴
[Tcl-bugs] [ tcl-Patches-585105 ] CONST-ified parser
|
From: <no...@so...> - 2002-08-02 16:29:55
|
Patches item #585105, was opened at 2002-07-22 16:02 You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=310894&aid=585105&group_id=10894 Category: 43. Parsing and Eval Group: CONSTify Status: Open Resolution: None Priority: 9 Submitted By: Don Porter (dgp) Assigned to: Joe English (jenglish) Summary: CONST-ified parser Initial Comment: I've put together a patch that constifies the Tcl parser routines, and things that call them, like Tcl_Eval(). First here is a patch for the existing files... ---------------------------------------------------------------------- >Comment By: Don Porter (dgp) Date: 2002-08-02 12:29 Message: Logged In: YES user_id=80530 Here's an alternate patch for Tk. It fully migrates Tk to use the Tcl 8.4 interface (no USE_COMPAT_CONST). The 3 trouble spots where Tk scribbles on command arguments are resolved in this patch by simple copying to Tcl_DStrings. If that hurts performance, it should be resolvable with a further conversion to new Tcl_ObjTypes representing event sequences and text indices. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2002-08-01 18:21 Message: Logged In: YES user_id=80530 There are 3 potential problems with the Tk patch. The Tcl Patch CONST-ifies the Tcl_CmdProc typedef, so that the string arguments passed in as "argv" are not supposed to be scribbled on. The Tk patch here makes use of USE_COMPAT_CONST to trick the compiler into using the 8.3 definition of Tcl_CmdProc, so Tk will build fine. However, if another module is built using the 8.4 definition, and the compiler relies on the "const" nature of the arguments to store them in read-only memory, there could be a run time error if in fact Tk does attempt to scribble on any of the arguments passed to its commands. Tk in fact does make 3 such attempts: generic/tkBind.c, lines 4218, 4222: parsing an event specifier generic/tkTextIndex.c, lines 368,370,457,459,479,481,491,493: parsing text index strings generic/tkTest.c, line 2259: some strange conversion of \n <-> \0 A better patch for Tk will address these areas. ---------------------------------------------------------------------- Comment By: miguel sofer (msofer) Date: 2002-07-31 08:57 Message: Logged In: YES user_id=148712 I have reviewed the tcl patch: A-OK under linux, which I guess covers all unices in this case. However, I did not test its functionality with extensions. Somebody else should look at: * the tk patch * Mac/Win compilation ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2002-07-29 21:48 Message: Logged In: YES user_id=68433 Oops, the patch is already out of date; there have been subsequent conflicting changes to tclVar.c. r1.61 fixed a bug that this patch also fixes, but in a different way. It (r1.61) also streamlined the final control flow, eliminating a 'goto'. r1.62 appears to be unrelated (at least there are no conflicts). Attached is a new patch to tclVar.c r1.62, incorporating the changes from 1.61 to 1.62 and the applicable changes from 1.60 to 1.61. ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2002-07-29 20:30 Message: Logged In: YES user_id=68433 Raising priority to make sure this gets considered for the 8.4b2 release (week of 5 Aug 2002). At least the parser and Tcl_Var* changes should be committed, even if the Tcl_CmdProc signature change is not. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2002-07-24 21:44 Message: Logged In: YES user_id=80530 Here's an updated patch that fixes those problems. Passing on to maintainer for further review. I will be away until July 31. Work out further fixes amongs yourselves please. ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2002-07-24 17:19 Message: Logged In: YES user_id=68433 Reviewed 'complete.patch': this looks very good. The parser changes should definitely be committed; Tcl_Eval() and friends finally have the right signature, the patch fixes at least one known bug and probably several as-yet-undiscovered ones, and the code is now simpler and better-factored. With the patch, I was finally able to fully-CONSTify an extension that used to require -DUSE_NON_CONST. Just a few minor notes: In generic/tclInt.h, TclCmdProcType should be changed to match Tcl_CmdProc: typedef Tcl_CmdProc *TclCmdProcType; With the current definition (int (*TclCmdProcType)(...CONST char **argv)), there may be a type mismatch in extensions that #include tclInt.h with -DUSE_NON_CONST . Also, there's some stray debugging code (commented-out fprintfs) that made its way into the patch, and a comment in tclParseExpr.c/ParsePrimaryExpr() (~line 1390) that is no longer accurate. Other than that, I didn't see any problems. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2002-07-23 23:52 Message: Logged In: YES user_id=80530 Here is a companion patch for Tk. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2002-07-23 23:51 Message: Logged In: YES user_id=80530 Here is an updated patch that completes CONST-ification of Tcl. It has a substantial parser re-write that operates on counted strings, and doesn't have the thread-safety issues of the last attempt. Also merged in are the features of Patch 582429 which CONST-ifies the Tcl_*Var* interfaces, and the patch attached to Bug 580433 that enhances the USE_NON_CONST macro to allows users of the C API to completely avoid all source incompatibilities with 8.3 headers if they need to. It's been said that CONST-ification needs to be an "all or nothing" affair. This patch lets us explore the "all" option. It's also been said that CONST-ification of the parser would kill performance. I don't see a significant problem with it. Simple testing with tclbench indicates the patched Tcl is overall faster than even the HEAD, and a still a significant improvement over 8.4b1 (thanks, MIguel!). There are some things that are slower, notably evaluation of unbraced [expr]'s, but haven't we advised against those for awhile now? Anyhow the main point was to get something working so that we can decide whether or not to adopt it based on measurements rather than speculation, so please measure away! Try it. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2002-07-22 16:04 Message: Logged In: YES user_id=80530 ...and here is a new file tclParse.h to place in the generic directory. Apply and `make genstubs', then try it. Note that the patched code will not be thread-safe. Further revisions required for that. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=310894&aid=585105&group_id=10894 |