develooper Front page | perl.perl5.porters | Postings from July 2013

[perl #118091] [GITHUB] Split gv_fetchpvn_flags into smaller functions

From:
Father Chrysostomos via RT
Date:
July 7, 2013 18:19
Subject:
[perl #118091] [GITHUB] Split gv_fetchpvn_flags into smaller functions
Message ID:
rt-3.6.HEAD-2552-1373221153-952.118091-15-0@perl.org
On Tue May 21 08:23:20 2013, Hugmeir wrote:
> This is a bug report for perl from fraserbn@gmail.com,
> generated with the help of perlbug 1.39 running under perl 5.16.2.
> 
> 
> -----------------------------------------------------------------
> [Please describe your issue here]
> 
> https://github.com/Hugmeir/utf8mess/commits/split_gv_fetchpvn_flags
> 
> That branch splits gv_fetchpvn_flags() into four helper functions:
> parse_gv_stash_name, which parses the name passed in to find a glob
> name
> and optionally a stash, find_default_stash, which, if the previous
> function
> didn't find a stash or some flags were passed in, looks for a suitable
> stash
> for the new glob, magicalize_gv, which adds magic to a newly-created
> glob,
> and
> maybe_multimagic_gv, which checks if an already-existing glob needs a
> different
> king of magic, ala *! needing to be magicalized for $! and %!.
> ...I'm not confident the names are all that descriptive, so feel free
> to
> suggest alternatives for those.
> 
> The branch also adds some comments to make better sense of how it
> all works.  They are nowhere near comprehensive and need a second
> pair of eyes for correctness, but I think that anything that clarifies
> gv_fetchpvn_flags is a good step forward.

Thank you.  But....

    else if (parse_gv_stash_name(&stash, &gv, &name, &len, nambeg,
full_len, is_utf8, add)) {

Eight-member parameter lists confuse me even more than large functions.
 If I am alone in this, fine.

I realise I am guilty of doing this, too, but solely to avoid code
duplication.

I think splitting less of the next part into find_default_stash and
having the latter (with a different name) just return a boolean
indicating whether an unqualified symbol belongs in the main package
would not only be clearer but also provide a useful API function for
modules to use, i.e., gv_is_in_main(name, namelen, utf8).  (The utf8
flag would, of course, be ignored, but should stay for completeness’
sake and possible future compatibility.)

I do *not* think magicalize_gv (should be named gv_magicalize) should be
adding the GV to the stash.  That is gv_fetchpvn_flags’ job.  Also,
having a gv_magicalize funcion that just vivifies members of the GV
passed in would be a useful API function.

I know addmg complicates things.  The problem is that Errno and
Tie::Hash::NamedCapture tie the variable when loaded, so the GV must
have been added to the stash already.  arybase just copied those two. 
Maybe we can change those modules not to do that, and have
gv_fetchpvn_flags tie the variable or pass the SV to the modules and
have them act on that instead of looking it up via a recursive
gv_fetchpvn_flags call.

(Maybe leave the addmg parameter in for now and deal with that last
paragraph later.)

-- 

Father Chrysostomos


---
via perlbug:  queue: perl5 status: open
https://rt.perl.org:443/rt3/Ticket/Display.html?id=118091



nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About