summaryrefslogtreecommitdiff
tag namecode-review/2009-07-22 (790fbd2b1de809897ebacddb828652a3f1d75dd9)
tag date2009-07-22 21:39:13 -0400
tagged byDavid Golden <dagolden@cpan.org>
tagged objectcommit a1834a8ee2...
downloadperl-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.