Correct comments about B::PV::PVBM.
authorNicholas Clark <nick@ccl4.org>
Tue, 17 May 2011 15:07:17 +0000 (16:07 +0100)
committerNicholas Clark <nick@ccl4.org>
Fri, 20 May 2011 10:37:25 +0000 (11:37 +0100)
Original comments, partially incorrect, added in 5a44e503dc748f53 and
f4c3658468ba5234.

ext/B/B.xs

index 627f851..f4d5fea 100644 (file)
@@ -1620,12 +1620,17 @@ PV(sv)
            len = 256;
        } else if (ix == 2) {
            /* This used to read 257. I think that that was buggy - should have
-              been 258. (The "\0", the flags byte, and 256 for the table.  Not
-              that anything anywhere calls this method.  NWC.  */
-           /* Also, the start pointer has always been SvPVX(sv). Surely it
-              should be SvPVX(sv) + SvCUR(sv)?  The code has faithfully been
-              refactored with this behaviour, since PVBM was added in
-              651aa52ea1faa806.  */
+              been 258. (The "\0", the flags byte, and 256 for the table.)
+              The only user of this method is B::Bytecode in B::PV::bsave.
+              I'm guessing that nothing tested the runtime correctness of
+              output of bytecompiled string constant arguments to index (etc).
+
+              Note the start pointer is and has always been SvPVX(sv), not
+              SvPVX(sv) + SvCUR(sv) PVBM was added in 651aa52ea1faa806, and
+              first used by the compiler in 651aa52ea1faa806. It's used to
+              get a "complete" dump of the buffer at SvPVX(), not just the
+              PVBM table. This permits the generated bytecode to "load"
+              SvPVX in "one" hit.  */
            p = SvPVX_const(sv);
            len = SvCUR(sv) + (SvVALID(sv) ? 256 + PERL_FBM_TABLE_OFFSET : 0);
        } else if (ix) {