tag name | code-review/2009-07-22 (790fbd2b1de809897ebacddb828652a3f1d75dd9) |
tag date | 2009-07-22 21:39:13 -0400 |
tagged by | David Golden <dagolden@cpan.org> |
tagged object | commit a1834a8ee2... |
download | perl-code-review/2009-07-22.tar.gz |
---|
Code Review from IRC
On #corehackers:
17:56 jjore> Did you mean to drop all the MAD support?
17:56 nperez> yeah, i was noticing that too
17:57 jjore> I thought MAD was rotting but I wouldn't remove it.
18:26 xdg> jjore, I dropped it because I didn't understand it and someone
here said to take it out
18:46 jdb> xdg: Larry said the MAD stuff should not be dropped, even if it
bitrots a bit: http://use.perl.org/~chromatic/journal/39125
18:51 xdg> jdb, noted. thanks.
18:54 xdg> reading that, I'll call what I did "bitrot". Since I don't
understand MAD, I don't know how to make it do the right thing
as I change the code around it, so it's out. That's different
than someone ripping all the MAD stuff out for the sake of
eliminating MAD
18:55 jdb> Maybe just leave the code in there, commented out with a short
explanation
18:55 jdb> That's easier than tracking down the missing bit in git later.
18:55 jdb> At least it gives someone who understands the MAD stuff a better
chance to see what might be missing
On #p5p
18:26 @jjore> xdg, in http://github.com/dagolden/perl/commit/5fdce45a7889979e70453fa36d2dd3aa1c73d808,
I'm not sure that package names can't have nulls in them.
18:26 +dipsy> urgh. long url. Try http://tinyurl.com/nus6p9
18:26 @jjore> though, never when parsed.
18:27 @xdg> jjore, that's a version number, which can't be null
18:28 @jjore> Ah. Long ago I asked on perl5-porters@ for Larry's definition of
each of the MAD tokens. You could use that.
18:28 @jjore> but I meant the package, not the version.
18:29 @jjore> Null handling in packages is problematic and not supported tho,
I think.
18:29 @jjore> Once, I said bless([], "\0") to bless at the '' package but
not get the ''->'main' conversion
18:29 @xdg> I'm not sure I understand the problem.
18:30 @jjore> *{"Devel::\0::Hi::VERSION"} = ...
18:30 @xdg> I think if PL_curstname is "\0", then package_version ends up
trying to set "$::VERSION"
18:32 @jjore> No, ${"\0::VERSION"}
18:33 @jjore> I feel like I'm bike-shedding though.
18:36 @jjore> If the current source code is utf8, presumably $VERSION
will be too
18:36 @jjore> and did you drop support for unversioned packages?
18:37 @jjore> maybe I'm not reading the grammar right.
18:48 @xdg> unversioned packages are just fine. $VERSION will be whatever
scan_version() parses, which is what "use MODULE VERSION" does,
so if that's broken for utf8, it's broken for both
18:48 @jjore> fascinating.
18:48 @jjore> I wonder if it is.
18:49 @jjore> or it's an interesting corner to poke at sometime.
18:49 @xdg> Since I'm doing sv_catpv(), I would think that if the *PV of
PL_curstnam is "\0", that it concatenates like the empty string
18:50 @xdg> Oh, I see. *PV is actually "\0\0" with LEN 1
18:51 @xdg> I'd suggest fixing that by banning \0 as a package name.
It's stupid in the first place.