On Tue, Sep 22, 2009 at 06:16:48PM -0700, Darren Duncan wrote:
> Now I recognize that Class::MOP provides these, and other, utility functions:
>
> Class::MOP::is_class_loaded($class_name)
> Class::MOP::load_class($class_name)
>
> ... and that these ostensibly are better used instead of doing it manually.
>
> However, from my looking through their source, it appears that these
> methods may not be as robust as I might desire.
>
> Part of my question to you is whether what I'm going to describe next is
> intentional behavior, or whether it is a candidate for CMOP enhancement.
> Or whether I've been expecting behavior that I ought not to be.
>
> 1. I looked for the implementation of is_class_loaded() and it seems the
> only implementation is XS, in the file MOP.xs, with no Pure Perl version
> existing, although the code comments with that XS says there is a Pure
> Perl version in lib/Class/MOP.pm which one should study to better
> understand the "grotty logic". Now I interpret this as that the code
> comment is outdated and the Pure Perl version was removed sometime around
> when it was decided to make a C compiler a mandatory requirement of using
> CMOP. If I'm wrong, please point out the Pure Perl version, or else the
> comment needs updating.
Yeah, that comment is outdated, for the reasons you mentioned.
> 2. From lib/Class/MOP.pm:
>
> sub _try_load_one_class {
> my $class = shift;
> return if is_class_loaded($class);
> my $file = _class_to_pmfile($class);
> return do {
> local $@;
> local $SIG{__DIE__};
> eval { require($file) };
> $@;
> };
> }
>
> One thing that I'm doing which CMOP doesn't seem to do is test after an
> error-free eval that the successfully compiled module actually declares a
> class of the requested name. For example, by adding another
> is_class_loaded() call after the "eval" line iff $@ doesn't hold an
> error. So the way CMOP is now, load_class() seems to return true if it
> compiles the appropriately named file, and regardless of whether the
> desired class was declared by it.
>
> That seems to be an error to me. Would you agree or disagree?
Not sure about this one... it sounds reasonable, but don't know if other
code might be relying on this?
> 3. I seem to recall reading somewhere, maybe in the Try::Tiny docs, that
> it is a common error to test $@ for truthiness rather than definedness.
> CMOP is just testing for truthiness, which would I think fail if someone
> throws an exception consisting of the number zero or the empty string.
>
> So that also seems to be an error in CMOP. Agree/disagree?
>
> P.S. Yes, I recognize my own code also has some of the same mistakes of
> #3, and I'll fix them.
Yeah, there are probably a bunch of places that we should be using
Try::Tiny rather than eval... this does look to be one of them.
-doy
