Improve mro merging error messages.
[gitmo/Class-C3-XS.git] / XS.xs
diff --git a/XS.xs b/XS.xs
index 6143c5e..94ba2d8 100644 (file)
--- a/XS.xs
+++ b/XS.xs
 
 #ifndef SvREFCNT_inc
 #  ifdef PERL_USE_GCC_BRACE_GROUPS
-#    define SvREFCNT_inc(sv)           \
-      ({                               \
-          SV * const _sv = (SV*)(sv);  \
-          if (_sv)                     \
-               (SvREFCNT(_sv))++;      \
-          _sv;                         \
+#    define SvREFCNT_inc(sv) \
+      ({ \
+          SV * const _sv = (SV*)(sv); \
+          if (_sv) \
+               (SvREFCNT(_sv))++; \
+          _sv; \
       })
 #  else
-#    define SvREFCNT_inc(sv)   \
+#    define SvREFCNT_inc(sv) \
           ((PL_Sv=(SV*)(sv)) ? (++(SvREFCNT(PL_Sv)),PL_Sv) : NULL)
 #  endif
 #endif
@@ -67,7 +67,6 @@ __mro_linear_isa_c3(pTHX_ HV* stash, HV* cache, I32 level)
     I32 made_mortal_cache = 0;
 
     assert(stash);
-    assert(HvAUX(stash));
 
     stashname = HvNAME(stash);
     stashname_len = strlen(stashname);
@@ -145,7 +144,9 @@ __mro_linear_isa_c3(pTHX_ HV* stash, HV* cache, I32 level)
                     SV* const seqitem = *seq_ptr++;
                     HE* const he = hv_fetch_ent(tails, seqitem, 0, 0);
                     if(!he) {
-                        hv_store_ent(tails, seqitem, newSViv(1), 0);
+                        if(!hv_store_ent(tails, seqitem, newSViv(1), 0)) {
+                            croak("failed to store value in hash");
+                        }
                     }
                     else {
                         SV* const val = HeVAL(he);
@@ -230,13 +231,22 @@ __mro_linear_isa_c3(pTHX_ HV* stash, HV* cache, I32 level)
             /* If we had candidates, but nobody won, then the @ISA
                hierarchy is not C3-incompatible */
             if(!winner) {
+                SV *errmsg;
+                I32 i;
                 /* we have to do some cleanup before we croak */
 
+                errmsg = newSVpvf("Inconsistent hierarchy during C3 merge of class '%s':\n\t"
+                                  "current merge results [\n", stashname);
+                for (i = 0; i <= av_len(retval); i++) {
+                    SV **elem = av_fetch(retval, i, 0);
+                    sv_catpvf(errmsg, "\t\t%"SVf",\n", SVfARG(*elem));
+                }
+                sv_catpvf(errmsg, "\t]\n\tmerging failed on '%"SVf"'", SVfARG(cand));
+
                 SvREFCNT_dec(retval);
                 Safefree(heads);
 
-                Perl_croak(aTHX_ "Inconsistent hierarchy during C3 merge of class '%s': "
-                    "merging failed on parent '%s'", stashname, SvPV_nolen(cand));
+                croak("%"SVf, SVfARG(errmsg));
             }
         }
     }
@@ -252,7 +262,9 @@ __mro_linear_isa_c3(pTHX_ HV* stash, HV* cache, I32 level)
 
     if(!made_mortal_cache) {
         SvREFCNT_inc(retval);
-        hv_store(cache, stashname, stashname_len, (SV*)retval, 0);
+        if(!hv_store(cache, stashname, stashname_len, (SV*)retval, 0)) {
+            croak("failed to store value in hash");
+        }
     }
 
     return retval;
@@ -267,10 +279,14 @@ __dopoptosub_at(const PERL_CONTEXT *cxstk, I32 startingblock) {
     return i;
 }
 
-STATIC SV*
-__nextcan(pTHX_ SV* self, I32 throw_nomethod)
+XS(XS_Class_C3_XS_nextcan);
+XS(XS_Class_C3_XS_nextcan)
 {
-    register I32 cxix;
+    dVAR; dXSARGS;
+
+    SV* self = ST(0);
+    const I32 throw_nomethod = SvIVX(ST(1));
+    register I32 cxix = cxstack_ix;
     register const PERL_CONTEXT *ccstack = cxstack;
     const PERL_SI *top_si = PL_curstackinfo;
     HV* selfstash;
@@ -289,10 +305,13 @@ __nextcan(pTHX_ SV* self, I32 throw_nomethod)
     GV* candidate = NULL;
     CV* cand_cv = NULL;
     const char *hvname;
-    I32 items;
+    I32 entries;
     HV* nmcache;
     HE* cache_entry;
     SV* cachekey;
+    I32 i;
+
+    SP -= items;
 
     if(sv_isobject(self))
         selfstash = SvSTASH(SvRV(self));
@@ -305,63 +324,65 @@ __nextcan(pTHX_ SV* self, I32 throw_nomethod)
     if (!hvname)
         Perl_croak(aTHX_ "Can't use anonymous symbol table for method lookup");
 
-    cxix = __dopoptosub_at(cxstack, cxstack_ix);
-
     /* This block finds the contextually-enclosing fully-qualified subname,
        much like looking at (caller($i))[3] until you find a real sub that
-       isn't ANON, etc */
-    for (;;) {
-        /* we may be in a higher stacklevel, so dig down deeper */
-        while (cxix < 0) {
-            if(top_si->si_type == PERLSI_MAIN)
-                Perl_croak(aTHX_ "next::method/next::can/maybe::next::method must be used in method context");
-            top_si = top_si->si_prev;
-            ccstack = top_si->si_cxstack;
-            cxix = __dopoptosub_at(ccstack, top_si->si_cxix);
-        }
+       isn't ANON, etc (also skips over pureperl next::method, etc) */
+    for(i = 0; i < 2; i++) {
+        cxix = __dopoptosub_at(ccstack, cxix);
+        for (;;) {
+            /* we may be in a higher stacklevel, so dig down deeper */
+            while (cxix < 0) {
+                if(top_si->si_type == PERLSI_MAIN)
+                    Perl_croak(aTHX_ "next::method/next::can/maybe::next::method must be used in method context");
+                top_si = top_si->si_prev;
+                ccstack = top_si->si_cxstack;
+                cxix = __dopoptosub_at(ccstack, top_si->si_cxix);
+            }
 
-        if(CxTYPE((PERL_CONTEXT*)(&ccstack[cxix])) != CXt_SUB
-          || (PL_DBsub && GvCV(PL_DBsub) && ccstack[cxix].blk_sub.cv == GvCV(PL_DBsub))) {
-            cxix = __dopoptosub_at(ccstack, cxix - 1);
-            continue;
-        }
+            if(CxTYPE((PERL_CONTEXT*)(&ccstack[cxix])) != CXt_SUB
+              || (PL_DBsub && GvCV(PL_DBsub) && ccstack[cxix].blk_sub.cv == GvCV(PL_DBsub))) {
+                cxix = __dopoptosub_at(ccstack, cxix - 1);
+                continue;
+            }
 
-        {
-            const I32 dbcxix = __dopoptosub_at(ccstack, cxix - 1);
-            if (PL_DBsub && GvCV(PL_DBsub) && dbcxix >= 0 && ccstack[dbcxix].blk_sub.cv == GvCV(PL_DBsub)) {
-                if(CxTYPE((PERL_CONTEXT*)(&ccstack[dbcxix])) != CXt_SUB) {
-                    cxix = dbcxix;
-                    continue;
+            {
+                const I32 dbcxix = __dopoptosub_at(ccstack, cxix - 1);
+                if (PL_DBsub && GvCV(PL_DBsub) && dbcxix >= 0 && ccstack[dbcxix].blk_sub.cv == GvCV(PL_DBsub)) {
+                    if(CxTYPE((PERL_CONTEXT*)(&ccstack[dbcxix])) != CXt_SUB) {
+                        cxix = dbcxix;
+                        continue;
+                    }
                 }
             }
-        }
 
-        cvgv = CvGV(ccstack[cxix].blk_sub.cv);
+            cvgv = CvGV(ccstack[cxix].blk_sub.cv);
 
-        if(!isGV(cvgv)) {
-            cxix = __dopoptosub_at(ccstack, cxix - 1);
-            continue;
-        }
+            if(!isGV(cvgv)) {
+                cxix = __dopoptosub_at(ccstack, cxix - 1);
+                continue;
+            }
 
-        /* we found a real sub here */
-        sv = sv_2mortal(newSV(0));
+            /* we found a real sub here */
+            sv = sv_2mortal(newSV(0));
 
-        gv_efullname3(sv, cvgv, NULL);
+            gv_efullname3(sv, cvgv, NULL);
 
-        fq_subname = SvPVX(sv);
-        fq_subname_len = SvCUR(sv);
+            fq_subname = SvPVX(sv);
+            fq_subname_len = SvCUR(sv);
 
-        subname = strrchr(fq_subname, ':');
-        if(!subname)
-            Perl_croak(aTHX_ "next::method/next::can/maybe::next::method cannot find enclosing method");
+            subname = strrchr(fq_subname, ':');
+            if(!subname)
+                Perl_croak(aTHX_ "next::method/next::can/maybe::next::method cannot find enclosing method");
 
-        subname++;
-        subname_len = fq_subname_len - (subname - fq_subname);
-        if(subname_len == 8 && strEQ(subname, "__ANON__")) {
-            cxix = __dopoptosub_at(ccstack, cxix - 1);
-            continue;
+            subname++;
+            subname_len = fq_subname_len - (subname - fq_subname);
+            if(subname_len == 8 && strEQ(subname, "__ANON__")) {
+                cxix = __dopoptosub_at(ccstack, cxix - 1);
+                continue;
+            }
+            break;
         }
-        break;
+        cxix--;
     }
 
     /* If we made it to here, we found our context */
@@ -377,9 +398,10 @@ __nextcan(pTHX_ SV* self, I32 throw_nomethod)
         if(val == &PL_sv_undef) {
             if(throw_nomethod)
                 Perl_croak(aTHX_ "No next::method '%s' found for %s", subname, hvname);
-            return &PL_sv_undef;
+            XSRETURN_EMPTY;
         }
-        return SvREFCNT_inc(val);
+        XPUSHs(sv_2mortal(newRV_inc(val)));
+        XSRETURN(1);
     }
 
     /* beyond here is just for cache misses, so perf isn't as critical */
@@ -390,20 +412,20 @@ __nextcan(pTHX_ SV* self, I32 throw_nomethod)
     linear_av = __mro_linear_isa_c3(aTHX_ selfstash, NULL, 0);
 
     linear_svp = AvARRAY(linear_av);
-    items = AvFILLp(linear_av) + 1;
+    entries = AvFILLp(linear_av) + 1;
 
-    while (items--) {
+    while (entries--) {
         SV* const linear_sv = *linear_svp++;
         assert(linear_sv);
         if(sv_eq(linear_sv, stashname))
             break;
     }
 
-    if(items > 0) {
+    if(entries > 0) {
         SV* sub_sv = sv_2mortal(newSVpv(subname, subname_len));
         HV* cc3_mro = get_hv("Class::C3::MRO", 0);
 
-        while (items--) {
+        while (entries--) {
             SV* const linear_sv = *linear_svp++;
             assert(linear_sv);
 
@@ -448,17 +470,22 @@ __nextcan(pTHX_ SV* self, I32 throw_nomethod)
             if (SvTYPE(candidate) == SVt_PVGV && (cand_cv = GvCV(candidate)) && !GvCVGEN(candidate)) {
                 SvREFCNT_dec(linear_av);
                 SvREFCNT_inc((SV*)cand_cv);
-                hv_store_ent(nmcache, newSVsv(cachekey), (SV*)cand_cv, 0);
-                return (SV*)cand_cv;
+                if (!hv_store_ent(nmcache, newSVsv(cachekey), (SV*)cand_cv, 0)) {
+                    croak("failed to store value in hash");
+                }
+                XPUSHs(sv_2mortal(newRV_inc((SV*)cand_cv)));
+                XSRETURN(1);
             }
         }
     }
 
     SvREFCNT_dec(linear_av);
-    hv_store_ent(nmcache, newSVsv(cachekey), &PL_sv_undef, 0);
+    if (!hv_store_ent(nmcache, newSVsv(cachekey), &PL_sv_undef, 0)) {
+        croak("failed to store value in hash");
+    }
     if(throw_nomethod)
         Perl_croak(aTHX_ "No next::method '%s' found for %s", subname, hvname);
-    return &PL_sv_undef;
+    XSRETURN_EMPTY;
 }
 
 XS(XS_Class_C3_XS_calculateMRO);
@@ -545,10 +572,14 @@ XS(XS_Class_C3_XS_calc_mdt)
     class_mro = __mro_linear_isa_c3(aTHX_ class_stash, cache, 0);
 
     our_c3mro = newHV();
-    hv_store(our_c3mro, "MRO", 3, (SV*)newRV_noinc((SV*)class_mro), 0);
+    if(!hv_store(our_c3mro, "MRO", 3, (SV*)newRV_noinc((SV*)class_mro), 0)) {
+        croak("failed to store value in hash");
+    }
 
     hv = get_hv("Class::C3::MRO", 1);
-    hv_store_ent(hv, classname, (SV*)newRV_noinc((SV*)our_c3mro), 0);
+    if(!hv_store_ent(hv, classname, (SV*)newRV_noinc((SV*)our_c3mro), 0)) {
+        croak("failed to store value in hash");
+    }
 
     methods = newHV();
 
@@ -567,7 +598,7 @@ XS(XS_Class_C3_XS_calc_mdt)
         }
 
         hv_iterinit(mro_stash);
-        while(he = hv_iternext(mro_stash)) {
+        while((he = hv_iternext(mro_stash))) {
             CV* code;
             SV* mskey;
             SV* msval;
@@ -592,73 +623,32 @@ XS(XS_Class_C3_XS_calc_mdt)
             orig = newSVsv(mro_class);
             sv_catpvn(orig, "::", 2);
             sv_catsv(orig, mskey);
-            hv_store(meth_hash, "orig", 4, orig, 0);
-            hv_store(meth_hash, "code", 4, newRV_inc((SV*)code), 0);
-            hv_store_ent(methods, mskey, newRV_noinc((SV*)meth_hash), 0);
+            if( !hv_store(meth_hash, "orig", 4, orig, 0)
+             || !hv_store(meth_hash, "code", 4, newRV_inc((SV*)code), 0)
+             || !hv_store_ent(methods, mskey, newRV_noinc((SV*)meth_hash), 0) ) {
+                croak("failed to store value in hash");
+            }
         }
     }
 
-    hv_store(our_c3mro, "methods", 7, newRV_noinc((SV*)methods), 0);
-    if(has_ovf) hv_store(our_c3mro, "has_overload_fallback", 21, SvREFCNT_inc(has_ovf), 0);
-    XSRETURN_EMPTY;
-}
-
-XS(XS_next_can);
-XS(XS_next_can)
-{
-    dVAR; dXSARGS;
-
-    SV* self = ST(0);
-    SV* methcv = __nextcan(aTHX_ self, 0);
-
-    PERL_UNUSED_VAR(items);
-
-    if(methcv == &PL_sv_undef) {
-        ST(0) = &PL_sv_undef;
+    if(!hv_store(our_c3mro, "methods", 7, newRV_noinc((SV*)methods), 0)) {
+        croak("failed to store value in hash");
     }
-    else {
-        ST(0) = sv_2mortal(newRV_inc(methcv));
+    if(has_ovf) {
+        if(!hv_store(our_c3mro, "has_overload_fallback", 21, SvREFCNT_inc(has_ovf), 0)) {
+            croak("failed to store value in hash");
+        }
     }
-
-    XSRETURN(1);
-}
-
-XS(XS_next_method);
-XS(XS_next_method)
-{
-    dMARK;
-    dAX;
-    SV* self = ST(0);
-    SV* methcv = __nextcan(aTHX_ self, 1);
-
-    PL_markstack_ptr++;
-    call_sv(methcv, GIMME_V);
+    XSRETURN_EMPTY;
 }
 
-XS(XS_maybe_next_method);
-XS(XS_maybe_next_method)
-{
-    dMARK;
-    dAX;
-    SV* self = ST(0);
-    SV* methcv = __nextcan(aTHX_ self, 0);
-
-    if(methcv == &PL_sv_undef) {
-        ST(0) = &PL_sv_undef;
-        XSRETURN(1);
-    }
-
-    PL_markstack_ptr++;
-    call_sv(methcv, GIMME_V);
-}
+MODULE = Class::C3::XS  PACKAGE = Class::C3::XS
 
-MODULE = Class::C3::XS PACKAGE = Class::C3::XS
+PROTOTYPES: DISABLED
 
 BOOT:
     newXS("Class::C3::XS::calculateMRO", XS_Class_C3_XS_calculateMRO, __FILE__);
     newXS("Class::C3::XS::_plsubgen", XS_Class_C3_XS_plsubgen, __FILE__);
     newXS("Class::C3::XS::_calculate_method_dispatch_table", XS_Class_C3_XS_calc_mdt, __FILE__);
-    newXS("next::can", XS_next_can, __FILE__);
-    newXS("next::method", XS_next_method, __FILE__);
-    newXS("maybe::next::method", XS_maybe_next_method, __FILE__);
+    newXS("Class::C3::XS::_nextcan", XS_Class_C3_XS_nextcan, __FILE__);