Also cc'd to tcltk@perl.org
https://www.nntp.perl.org/group/perl.tcltk/2018/07/msg718.html
per Vadim's request
At 09:07 AM 7/3/2018, you wrote:
<URL: https://rt.cpan.org/Ticket/Display.html?id=125472 >
Thank you for the prompt and detailed response.
> I added comments to the tests i added to t/ to document the problems
> i noticed with the v1.05 process that you may find helpful to read.
>
> I found that others were already using the create_tcl_sub() entry
> even though it was not documented. And i found that the description
> of CreateCommnd() to be confusing to me as well.
I think someone uses the internal function does not necessary mean
that the function has to be exposed by document. The difference is:
If a module user uses undocumented internal function, he/she takes
the risk of function robustness and compatibility on module
revision. On the other hand, if a module document a function as an
API, the module shall take response that both interface and behavior
should not change in the future.
I agree that documented interfaces should continue to work as
documented, but that does not mean they must stay static, it just
means they should not bust any pervious uses.
After looking at tcl.pm/.xs it was clear to me that create_tcl_sub()
should be the preferred method for introducing code to tcl because
CreateCommand() was so vaguely documented and its arguments are very
specific to the tcl interface and can cause bigger problems is used
improperly.
create_tcl_sub() takes care of much of the needed background
processing needed to properly create a tcl<=>perl subroutine such as
placeing it into the proper namespace and returing the tcl name in
that namespace. it also populates the tracking structure such that
further uses of a static coderef will not repopulate the tcl
subroutine internal reference (in v1.02- and v1.06+ at least) if it
is found a second time by call().
> Since create_tcl_sub() is what was used in call(), and that was that
> pair where the code needed to be patched i thought it made sense to
> document how create_tcl_sub() worked and how code disposal worked
> within it. IMHO it made sense to expose create_tcl_sub() so that
> programmers could use the automatic recoding of a call(,,$coderef)
> process as well as being able to control when code disposal took
> place if they wanted to handle it themselves. This allows them to
> continue to use coderefs if they want and just delete one use by
> using DESCRNAME, or all clear it totally by using TCLNAME.
Yes, it makes sense to document how internal mechanism works, but
again it does not mean create_tcl_sub() has to be documented so
detail and as a part of API.
> I think i made it clear that the user supplied DESCRNAME can be
> anything they want it to be, as long as it does not begin with '='
> or '@', or '::perl::'.
> '=' now prefaces "normal" entries created by create_tcl_sub().
> '@' prefaces entries create_tcl_sub() will use to possibly dispose of
> coderefs "AT" a future time since they were used by an after command.
> and '::perl::' prefaces the TCLNAME assigned by create_tcl_sub() and
> is used in anon_refs{} to determine when no more "users" exist and
> the coderef can now be safely deleted from the tcl namespace.
Okay, I finally get the the point of DESCRNAME usage now. I did not
know that DESCRNAME can be passed to _code_dispose() so I did not
get the purpose of DESCRNAME at all. I suggest to have more explicit
description that argument of _code_dispose() is either TCLNAME or
DESCRNAME. And the major purpose of DESCRNAME is used to pass to
_code_disposal().
I get your point more, but I still think we shall properly define
the interface to support code disposal instead of just directly
expose an internal function out. For example, how about modify
call() to support the form of call(DESCRNAME, cmd, arg ...), where
DESCRNAME is optional and is a string that with '@' prefix. Then we
have a good compatibility and user do not have to worry about
GENCODE, EVENTS etc.
The main purpose of DESCRNAME is to name the slot used in $anon_refs
to store information about the tcl<=>perl subroutine linkage and
track it. Just one of the purposes of tracking is to be able to
dispose of it later.
I do not understand why you mean to accomplish by modifying call()
that way, how would call understand that it is a descrname and not a tcl verb?
The @ prefix was selected to identify a group of coderefs that
might be able to be deleted by a call() generated "after" call to
_code_dispose once the initiating "after" call has completed. For
the most part $anon_refs keys beginning with '@" should be very short
lived transients. I think adding another use for the '@' prefix would
be counterproductive. And a call() can also have more than one
coderef in its args as call('if','1',$sub1,$sub2) illustrates.
The documentation for Tcl::_code_destroy(NAME) already states
>calling _code_destroy on a TCLNAME retruned from create_tcl_sub
removes all use instances and purges the command table.
>calling _code_destroy on a DESCRNAME passed to create_tcl_sub
removes only that instace
>Code used in a DESCRNAME may be used in other places as well,
>only when the last usage is purged does the entry get purged from
the command table
I think that is a pretty good description of its modes, but if you
can suggest a better description you are free to have a go at it.
I can understand why you think that EVENTS should be hidden better,
and then GENCODE as well, but both of them were well integrated into
the existing code and i did not want to break that. I can also see
cases where a user may want to use the EVENTS parm tho, and in that
case they would want both the TCLNAME and GENCODE returned, so i
introduced a change to what was returned to allow for that. Existing
code expecting a scalar to be returned still gets GENCODE as it did
before, but new code expecting an array can get both of them. And
uless EVENTS was populated both are the same anyway.
> > 2) The newest POD in github HEAD mentioned
> >
> > $interp->call('fileevent','sock9827430','writable');
> >
> > won't dispose the sub created from
> >
> > $interp->call('fileevent','sock9827430', 'writable'=>sub{...});
> >
> > is it still true? Or only 'set' command behaves in this way?
>
> yes this is still true, it was true in v1.02 and v1.05 and reverted
> back to that behavior in v1.11. at v1.06 i had tried to make it so
> the first call would cause code disposal of the sub created by the
> second, but that was what broke your "set foo" tests. At the time i
> introduced that behavior i thought it made sense to try to initiate
> code disposal in those cases, to keep the internal table and tcl
> namespace clean, but after spending some time trying to decide if
> "set foo" was the only place this should be avoided i decided that
> it was safer to revert to the original processing where code
> disposal would only be triggered if there was a different coderef in
> the new call. but not when the coderef was now missing.
>
> I wrapped the place where it occurs with a flag so it could be
> reinstated if desired when it was determined that only "set foo"
> should be handled in this special manner
Too bad that it is disabled by default. IMO
'$interp->call("FILEEVENT" ... "WRITABLE=>"")' shall be a suggested
way to dispose the code and well documented. 'set' command is an
exception and to be documented. Of course there are other exceptions
to be found out. The flag SAVENOCODE might be better documented in POD too.
i did not document SAVENOCODE nor SAVEALLCODES because i feel both to
be transient flags destined to be deleted when the tcl.pm code is solidified.
both serve to temporarily encapsulate code i felt to be useful but
might not deserve to be in the final code, and allow the encapsulated
code to be toggled on/off to see how it works in different
situations. If you look at where both are used i think you will find
that i did document what i was trying to do when i introduced the
code they encapsulate. Again if you think you can explain it better
please feel free to do so.
i am not a fan of "Of course there are other exceptions to be found
out." methods. It disgusted me greatly that my introduction of the
code to deal with '$interp->call("FILEEVENT" ... "WRITABLE=>"")' (or
its bind cousins) busted the "set foo" test in Tkx.pm. I hate to
bust existing working code and feel it diminishes my reputation as a
responsible programmer when i do so.
I would rather shut it off for now, reverting to the existing process
in v1.02/v1.05 than to play catchup to bug reports stating it busted
something else. I have been thru much of the tcl/tk documentation
looking for other cases the newer behavior may also break but so far
have only identified the "set foo" instance, but that does not mean
no other cases still exist and would rather revert to the previous
behavior until i can be assured that the "set foo" instance is the
only one to be exempted rather than bust someone elses code and
require them to figure out how to file a bug report about it. When
we are assured that the code will do what we want it is a simple
matter to remove the encapsulation and delete the transient flags.
SJ
This new code disposal process is opening a can of worms, but that
does not mean it cannot be accomplished successfully. As i was
working on the reference counting i imagined inserting a new code
disposal method that could clean up after a deleted tk element,
searching the keys of %anon_refs for keys prefixed with '='
(internally generated) and tracking button/bind/menu objects related
to a element name specified as a parameter. It could then dispose of
them in the normal method, freeing any that are no longer referenced.
while i did not think i should being this up until the code disposal
code has stabilized more, i think now is not a bad time to mention it
as it serves to solidify the direction i think code_disposal should
proceed in.
