diff options
Diffstat (limited to 'generic/tclEnsemble.c')
| -rw-r--r-- | generic/tclEnsemble.c | 268 |
1 files changed, 122 insertions, 146 deletions
diff --git a/generic/tclEnsemble.c b/generic/tclEnsemble.c index b55489b..bf3196d 100644 --- a/generic/tclEnsemble.c +++ b/generic/tclEnsemble.c @@ -70,8 +70,8 @@ enum EnsConfigOpts { }; /* - * This structure defines a Tcl object type that contains a reference to an - * ensemble subcommand (e.g. the "length" in [string length ab]). It is used + * ensembleCmdType is a Tcl object type that contains a reference to an + * ensemble subcommand, e.g. the "length" in [string length ab]. It is used * to cache the mapping between the subcommand itself and the real command * that implements it. */ @@ -1704,7 +1704,7 @@ NsEnsembleImplementationCmdNR( size_t subIdx; /* - * Must recheck objc, since numParameters might have changed. Cf. test + * Must recheck objc since numParameters might have changed. See test * namespace-53.9. */ @@ -1712,7 +1712,7 @@ NsEnsembleImplementationCmdNR( subIdx = 1 + ensemblePtr->numParameters; if ((size_t)objc < subIdx + 1) { /* - * We don't have a subcommand argument. Make error message. + * No subcommand argument. Make error message. */ Tcl_DString buf; /* Message being built */ @@ -1744,18 +1744,16 @@ NsEnsembleImplementationCmdNR( } /* - * Determine if the table of subcommands is right. If so, we can just look - * up in there and go straight to dispatch. + * If the table of subcommands is valid just lookup up the command there + * and go to dispatch. */ subObj = objv[subIdx]; if (ensemblePtr->epoch == ensemblePtr->nsPtr->exportLookupEpoch) { /* - * Table of subcommands is still valid; therefore there might be a - * valid cache of discovered information which we can reuse. Do the - * check here, and if we're still valid, we can jump straight to the - * part where we do the invocation of the subcommand. + * Table of subcommands is still valid so if the internal representtion + * is an ensembleCmd, just call it. */ EnsembleCmdRep *ensembleCmd; @@ -1777,8 +1775,8 @@ NsEnsembleImplementationCmdNR( } /* - * Look in the hashtable for the subcommand name; this is the fastest way - * of all if there is no cache in operation. + * Look in the hashtable for the named subcommand. This is the fastest + * path if there is no cache in operation. */ hPtr = Tcl_FindHashEntry(&ensemblePtr->subcommandTable, @@ -1786,26 +1784,25 @@ NsEnsembleImplementationCmdNR( if (hPtr != NULL) { /* - * Cache for later in the subcommand object. + * Cache ensemble in the subcommand object for later. */ MakeCachedEnsembleCommand(subObj, ensemblePtr, hPtr, NULL); } else if (!(ensemblePtr->flags & TCL_ENSEMBLE_PREFIX)) { /* - * Could not map, no prefixing, go to unknown/error handling. + * Could not map. No prefixing. Go to unknown/error handling. */ goto unknownOrAmbiguousSubcommand; } else { /* - * If we've not already confirmed the command with the hash as part of - * building our export table, we need to scan the sorted array for - * matches. + * If the command isn't yet confirmed with the hash as part of building + * the export table, scan the sorted array for matches. */ - const char *subcmdName; /* Name of the subcommand, or unique prefix of - * it (will be an error for a non-unique - * prefix). */ + const char *subcmdName; /* Name of the subcommand or unique prefix of + * it (a non-unique prefix produces an error). + */ char *fullName = NULL; /* Full name of the subcommand. */ size_t stringLength, i; size_t tableLength = ensemblePtr->subcommandTable.numEntries; @@ -1820,10 +1817,10 @@ NsEnsembleImplementationCmdNR( if (cmp == 0) { if (fullName != NULL) { /* - * Since there's never the exact-match case to worry about - * (hash search filters this), getting here indicates that - * our subcommand is an ambiguous prefix of (at least) two - * exported subcommands, which is an error case. + * Hash search filters out the exact-match case, so getting + * here indicates that the subcommand is an ambiguous + * prefix of at least two exported subcommands, which is an + * error case. */ goto unknownOrAmbiguousSubcommand; @@ -1831,9 +1828,8 @@ NsEnsembleImplementationCmdNR( fullName = ensemblePtr->subcommandArrayPtr[i]; } else if (cmp < 0) { /* - * Because we are searching a sorted table, we can now stop - * searching because we have gone past anything that could - * possibly match. + * The table is sorted so stop searching because a match would + * have been found already. */ break; @@ -1841,7 +1837,7 @@ NsEnsembleImplementationCmdNR( } if (fullName == NULL) { /* - * The subcommand is not a prefix of anything, so bail out! + * The subcommand is not a prefix of anything. Bail out! */ goto unknownOrAmbiguousSubcommand; @@ -1871,21 +1867,19 @@ NsEnsembleImplementationCmdNR( runResultingSubcommand: /* - * Do the real work of execution of the subcommand by building an array of - * objects (note that this is potentially not the same length as the - * number of arguments to this ensemble command), populating it and then - * feeding it back through the main command-lookup engine. In theory, we - * could look up the command in the namespace ourselves, as we already - * have the namespace in which it is guaranteed to exist, + * Execute the subcommand by populating an array of objects, which might + * not be the same length as the number of arguments to this ensemble + * command, and then handing it to the main command-lookup engine. In + * theory, the command could be looked up right here using the namespace in + * which it is guaranteed to exist, * * ((Q: That's not true if the -map option is used, is it?)) * - * but we don't do that (the cacheing of the command object used should - * help with that.) + * but don't do that because cacheing of the command object should help. */ { - Tcl_Obj *copyPtr; /* The actual list of words to dispatch to. + Tcl_Obj *copyPtr; /* The list of words to dispatch on. * Will be freed by the dispatch engine. */ Tcl_Obj **copyObjv; int copyObjc, prefixObjc; @@ -1908,8 +1902,8 @@ NsEnsembleImplementationCmdNR( TclDecrRefCount(prefixObj); /* - * Record what arguments the script sent in so that things like - * Tcl_WrongNumArgs can give the correct error message. Parameters + * Record the words of the command as given so that routines like + * Tcl_WrongNumArgs can produce the correct error message. Parameters * count both as inserted and removed arguments. */ @@ -1931,10 +1925,9 @@ NsEnsembleImplementationCmdNR( unknownOrAmbiguousSubcommand: /* - * Have not been able to match the subcommand asked for with a real - * subcommand that we export. See whether a handler has been registered - * for dealing with this situation. Will only call (at most) once for any - * particular ensemble invocation. + * The named subcommand did not match any exported command. If there is a + * handler registered unknown subcommands, call it, but not more than once + * for this call. */ if (ensemblePtr->unknownHandler != NULL && reparseCount++ < 1) { @@ -1950,10 +1943,10 @@ NsEnsembleImplementationCmdNR( } /* - * We cannot determine what subcommand to hand off to, so generate a - * (standard) failure message. Note the one odd case compared with - * standard ensemble-like command, which is where a namespace has no - * exported commands at all... + * Could not find a routine for the named subcommand so generate a standard + * failure message. The one odd case compared with a standard + * ensemble-like command is where a namespace has no exported commands at + * all... */ Tcl_ResetResult(interp); @@ -2000,8 +1993,8 @@ TclClearRootEnsemble( * * TclInitRewriteEnsemble -- * - * Applies a rewrite of arguments so that an ensemble subcommand will - * report error messages correctly for the overall command. + * Applies a rewrite of arguments so that an ensemble subcommand + * correctly reports any error messages for the overall command. * * Results: * Whether this is the first rewrite applied, a value which must be @@ -2079,7 +2072,7 @@ TclResetRewriteEnsemble( * * TclSpellFix -- * - * Record a spelling correction that needs making in the generation of + * Records a spelling correction that needs making in the generation of * the WrongNumArgs usage message. * * Results: @@ -2144,8 +2137,8 @@ TclSpellFix( if (badIdx < iPtr->ensembleRewrite.numInsertedObjs) { /* - * Misspelled value was inserted. We cannot directly jump to the bad - * value, but have to search. + * Misspelled value was inserted. Cannot directly jump to the bad + * value. Must search. */ idx = 1; @@ -2257,22 +2250,22 @@ TclFetchEnsembleRoot( /* * ---------------------------------------------------------------------- * - * EnsmebleUnknownCallback -- + * EnsembleUnknownCallback -- * - * Helper for the ensemble engine that handles the procesing of unknown - * callbacks. See the user documentation of the ensemble unknown handler - * for details; this function is only ever called when such a function is - * defined, and is only ever called once per ensemble dispatch (i.e. if a - * reparse still fails, this isn't called again). + * Helper for the ensemble engine. Calls the routine registered for + * "ensemble unknown" case. See the user documentation of the + * ensemble unknown handler for details. Only called when such a + * function is defined, and is only called once per ensemble dispatch. + * I.e. even if a reparse still fails, this isn't called again. * * Results: * TCL_OK - *prefixObjPtr contains the command words to dispatch * to. - * TCL_CONTINUE - Need to reparse (*prefixObjPtr is invalid). - * TCL_ERROR - Something went wrong! Error message in interpreter. + * TCL_CONTINUE - Need to reparse, i.e. *prefixObjPtr is invalid + * TCL_ERROR - Something went wrong. Error message in interpreter. * * Side effects: - * Calls the Tcl interpreter, so arbitrary. + * Arbitrary, due to evaluation of script provided by client. * * ---------------------------------------------------------------------- */ @@ -2289,7 +2282,7 @@ EnsembleUnknownCallback( Tcl_Obj **paramv, *unknownCmd, *ensObj; /* - * Create the unknown command callback to determine what to do. + * Create the "unknown" command callback to determine what to do. */ unknownCmd = Tcl_DuplicateObj(ensemblePtr->unknownHandler); @@ -2303,10 +2296,9 @@ EnsembleUnknownCallback( Tcl_IncrRefCount(unknownCmd); /* - * Now call the unknown handler. (We don't bother NRE-enabling this; deep - * recursing through unknown handlers is horribly perverse.) Note that it - * is always an error for an unknown handler to delete its ensemble; don't - * do that! + * Call the "unknown" handler. No attempt to NRE-enable this as deep + * recursion through unknown handlers is perverse. It is always an error + * for an unknown handler to delete its ensemble. Don't do that. */ Tcl_Preserve(ensemblePtr); @@ -2324,10 +2316,9 @@ EnsembleUnknownCallback( Tcl_Release(ensemblePtr); /* - * If we succeeded, we should either have a list of words that form the - * command to be executed, or an empty list. In the empty-list case, the - * ensemble is believed to be updated so we should ask the ensemble engine - * to reparse the original command. + * On success the result is a list of words that form the command to be + * executed. If the list is empty, the ensemble should have been updated, + * so ask the ensemble engine to reparse the original command. */ if (result == TCL_OK) { @@ -2336,11 +2327,7 @@ EnsembleUnknownCallback( TclDecrRefCount(unknownCmd); Tcl_ResetResult(interp); - /* - * Namespace is still there. Check if the result is a valid list. If - * it is, and it is non-empty, that list is what we are using as our - * replacement. - */ + /* A non-empty list is the replacement command. */ if (TclListObjLength(interp, *prefixObjPtr, &prefixObjc) != TCL_OK) { TclDecrRefCount(*prefixObjPtr); @@ -2353,7 +2340,7 @@ EnsembleUnknownCallback( } /* - * Namespace alive & empty result => reparse. + * Empty result => reparse. */ TclDecrRefCount(*prefixObjPtr); @@ -2361,7 +2348,7 @@ EnsembleUnknownCallback( } /* - * Oh no! An exceptional result. Convert to an error. + * Convert exceptional result to an error. */ if (!Tcl_InterpDeleted(interp)) { @@ -2401,16 +2388,16 @@ EnsembleUnknownCallback( * * MakeCachedEnsembleCommand -- * - * Cache what we've computed so far; it's not nice to repeatedly copy - * strings about. Note that to do this, we start by deleting any old - * representation that there was (though if it was an out of date - * ensemble rep, we can skip some of the deallocation process.) + * Caches what has been computed so far to minimize string copying. + * Starts by deleting any existing representation but reusing the existing + * structure if it is an ensembleCmd. * * Results: - * None + * None. * * Side effects: - * Alters the internal representation of the first object parameter. + * Converts the internal representation of the given object to an + * ensembleCmd. * *---------------------------------------------------------------------- */ @@ -2432,8 +2419,7 @@ MakeCachedEnsembleCommand( } } else { /* - * Kill the old internal rep, and replace it with a brand new one of - * our own. + * Replace any old internal representation with a new one. */ ensembleCmd = (EnsembleCmdRep *)Tcl_Alloc(sizeof(EnsembleCmdRep)); @@ -2459,17 +2445,16 @@ MakeCachedEnsembleCommand( * * DeleteEnsembleConfig -- * - * Destroys the data structure used to represent an ensemble. This is - * called when the ensemble's command is deleted (which happens - * automatically if the ensemble's namespace is deleted.) Maintainers - * should note that ensembles should be deleted by deleting their - * commands. + * Destroys the data structure used to represent an ensemble. Called when + * the procedure for the ensemble is deleted, which happens automatically + * if the namespace for the ensemble is deleted. Deleting the procedure + * for an ensemble is the right way to initiate cleanup. * * Results: * None. * * Side effects: - * Memory is (eventually) deallocated. + * Memory is eventually deallocated. * *---------------------------------------------------------------------- */ @@ -2501,10 +2486,7 @@ DeleteEnsembleConfig( EnsembleConfig *ensemblePtr = (EnsembleConfig *)clientData; Namespace *nsPtr = ensemblePtr->nsPtr; - /* - * Unlink from the ensemble chain if it has not been marked as having been - * done already. - */ + /* Unlink from the ensemble chain if it not already marked as unlinked. */ if (ensemblePtr->next != ensemblePtr) { EnsembleConfig *ensPtr = (EnsembleConfig *) nsPtr->ensembles; @@ -2530,7 +2512,7 @@ DeleteEnsembleConfig( ensemblePtr->flags |= ENSEMBLE_DEAD; /* - * Kill the pointer-containing fields. + * Release the fields that contain pointers. */ ClearTable(ensemblePtr); @@ -2548,10 +2530,9 @@ DeleteEnsembleConfig( } /* - * Arrange for the structure to be reclaimed. Note that this is complex - * because we have to make sure that we can react sensibly when an - * ensemble is deleted during the process of initialising the ensemble - * (especially the unknown callback.) + * Arrange for the structure to be reclaimed. This is complex because it is + * necessary to react sensibly when an ensemble is deleted during its + * initialisation, particularly in the case of an unknown callback. */ Tcl_EventuallyFree(ensemblePtr, TCL_DYNAMIC); @@ -2562,11 +2543,11 @@ DeleteEnsembleConfig( * * BuildEnsembleConfig -- * - * Create the internal data structures that describe how an ensemble - * looks, being a hash mapping from the full command name to the Tcl list - * that describes the implementation prefix words, and a sorted array of - * all the full command names to allow for reasonably efficient - * unambiguous prefix handling. + * Creates the internal data structures that describe how an ensemble + * looks. The structures are a hash map from the full command name to the + * Tcl list that describes the implementation prefix words, and a sorted + * array of all the full command names to allow for reasonably efficient + * handling of an unambiguous prefix. * * Results: * None. @@ -2574,7 +2555,7 @@ DeleteEnsembleConfig( * Side effects: * Reallocates and rebuilds the hash table and array stored at the * ensemblePtr argument. For large ensembles or large namespaces, this is - * a potentially expensive operation. + * may be an expensive operation. * *---------------------------------------------------------------------- */ @@ -2583,9 +2564,8 @@ static void BuildEnsembleConfig( EnsembleConfig *ensemblePtr) { - Tcl_HashSearch search; /* Used for scanning the set of commands in - * the namespace that backs up this - * ensemble. */ + Tcl_HashSearch search; /* Used for scanning the commands in + * the namespace for this ensemble. */ size_t i, j; int isNew; Tcl_HashTable *hash = &ensemblePtr->subcommandTable; @@ -2603,13 +2583,13 @@ BuildEnsembleConfig( /* * There is a list of exactly what subcommands go in the table. - * Must determine the target for each. + * Determine the target for each. */ Tcl_ListObjGetElements(NULL, subList, &subc, &subv); if (subList == mapDict) { /* - * Strange case where explicit list of subcommands is same value + * Unusual case where explicit list of subcommands is same value * as the dict mapping to targets. */ @@ -2658,10 +2638,10 @@ BuildEnsembleConfig( } /* - * target was not in the dictionary so map onto the namespace. - * Note in this case that we do not guarantee that the command - * is actually there; that is the programmer's responsibility - * (or [::unknown] of course). + * Target was not in the dictionary. Map onto the namespace. + * In this case there is no guarantee that the command + * is actually there. It is the responsibility of the + * programmer (or [::unknown] of course) to provide the procedure. */ cmdObj = Tcl_NewStringObj(name, -1); @@ -2672,9 +2652,9 @@ BuildEnsembleConfig( } } else if (mapDict) { /* - * No subcmd list, but we do have a mapping dictionary so we should - * use the keys of that. Convert the dictionary's contents into the - * form required for the ensemble's internal hashtable. + * No subcmd list, but there is a mapping dictionary, so + * use the keys of that. Convert the contents of the dictionary into the + * form required for the internal hashtable of the ensemble. */ Tcl_DictSearch dictSearch; @@ -2693,18 +2673,15 @@ BuildEnsembleConfig( } } else { /* - * Discover what commands are actually exported by the namespace. - * What we have is an array of patterns and a hash table whose keys - * are the command names exported by the namespace (the contents do - * not matter here.) We must find out what commands are actually - * exported by filtering each command in the namespace against each of - * the patterns in the export list. Note that we use an intermediate - * hash table to make memory management easier, and because that makes - * exact matching far easier too. + * Use the array of patterns and the hash table whose keys are the + * commands exported by the namespace. The corresponding values do not + * matter here. Filter the commands in the namespace against the + * patterns in the export list to find out what commands are actually + * exported. Use an intermediate hash table to make memory management + * easier and to make exact matching much easier. * - * Suggestion for future enhancement: compute the unique prefixes and - * place them in the hash too, which should make for even faster - * matching. + * Suggestion for future enhancement: Compute the unique prefixes and + * place them in the hash too for even faster matching. */ hPtr = Tcl_FirstHashEntry(&ensemblePtr->nsPtr->cmdTable, &search); @@ -2747,24 +2724,24 @@ BuildEnsembleConfig( } /* - * Create a sorted array of all subcommands in the ensemble; hash tables + * Create a sorted array of all subcommands in the ensemble. Hash tables * are all very well for a quick look for an exact match, but they can't - * determine things like whether a string is a prefix of another (not - * without lots of preparation anyway) and they're no good for when we're - * generating the error message either. + * determine things like whether a string is a prefix of another, at least + * not without a lot of preparation, and they're not useful for generating + * the error message either. * - * We do this by filling an array with the names (we use the hash keys - * directly to save a copy, since any time we change the array we change - * the hash too, and vice versa) and running quicksort over the array. + * Do this by filling an array with the names: Use the hash keys + * directly to save a copy since any time we change the array we change + * the hash too, and vice versa, and run quicksort over the array. */ ensemblePtr->subcommandArrayPtr = (char **)Tcl_Alloc(sizeof(char *) * hash->numEntries); /* - * Fill array from both ends as this makes us less likely to end up with - * performance problems in qsort(), which is good. Note that doing this - * makes this code much more opaque, but the naive alternatve: + * Fill the array from both ends as this reduces the likelihood of + * performance problems in qsort(). This makes this code much more opaque, + * but the naive alternatve: * * for (hPtr=Tcl_FirstHashEntry(hash,&search),i=0 ; * hPtr!=NULL ; hPtr=Tcl_NextHashEntry(&search),i++) { @@ -2772,11 +2749,11 @@ BuildEnsembleConfig( * } * * can produce long runs of precisely ordered table entries when the - * commands in the namespace are declared in a sorted fashion (an ordering - * some people like) and the hashing functions (or the command names - * themselves) are fairly unfortunate. By filling from both ends, it - * requires active malice (and probably a debugger) to get qsort() to have - * awful runtime behaviour. + * commands in the namespace are declared in a sorted fashion, which is an + * ordering some people like, and the hashing functions or the command + * names themselves are fairly unfortunate. Filling from both ends means + * that it requires active malice, and probably a debugger, to get qsort() + * to have awful runtime behaviour. */ i = 0; @@ -2802,8 +2779,7 @@ BuildEnsembleConfig( * * NsEnsembleStringOrder -- * - * Helper function to compare two pointers to two strings for use with - * qsort(). + * Helper to for uset with sort() that compares two string pointers. * * Results: * -1 if the first string is smaller, 1 if the second string is smaller, |