Scalars vs globs (*{} should not return FAKE globs) #10625
Comments
From @cpansproutThere are various bugs in perl related to the fact that globs are scalars. Various Perl operators need to distinguish between globs and scalars, even though the former are a subset of the latter. Now, there are two types of globs: autovivified globs (I call these ‘real globs’) and copies of globs (fake globs or glob copies). The first kind, a real glob, is what you get when you write *foo (prior stash manipulation aside). A real glob is always a glob. That sv is never downgraded to anything else. Even passing it to undef does not undefine it, but only its contents. The second occurs when you write ‘$a = *foo’. These are marked with the SVf_FAKE flag. I think the original purpose of the distinction was to allow $a to be assigned something other than a glob after it had been assigned a glob. Now different operators make the distinction between globs and scalar in different ways. For example, the = operator (in 5.12 and earlier) or, rather, sv_setsv_flags treats only real globs on the LHS as globs. Fake globs are scalars. Other operators, such as tie(d), untie and *{}, treat any glob as a glob, and not as a scalar. Now the *{} dereference operator returns its operand if it sees it as a glob. This means that (The first of those three examples is fixed in blead with a workaround for the actual problem. We now treat *{...}= as a special kind of assignment operator. List assignment is still broken.) If the patch for #77362 (glob-to-lvalue assignment) is applied, then there will be this problem, too: (*$pvlv_glob) = []; This innocent-looking statement will start assigning [] to $pvlv_glob, instead of the array slot of the GP. (Currently globs are stringified when assigned to PVLVs.) Now the real problem here seems to be that *{} is returning glob copies. We could make this operator return GvEGV(sv) in the presence of the SVf_FAKE flag, and make all operators that need to distinguish between globs and scalars check the SVf_FAKE flag. This would fix all these bugs in one fell swoop: (You can consider this a meta-ticket for all those.) (Some of those would actually still need fixing, but the fix would be to check a single flag, instead of the more complex, more intrusive patches already in some of those tickets.) There are two problems that can result: • Putting a fake glob in the symbol table will have the following effect: $::{foo} = *bar; # *foo is now fake This can be solved easily by making *{} turn the fake flag off if it is doing a symbolic dereference (or the compile-time equivalent). Having fake globs in the symbol table is questionable anyway. • This code: $foo_copy = *foo; will make *foo (aka GvEGV($foo_copy)) a copy of *bar, instead of making $foo_copy a copy of *bar. This is the most serious problem. The only solution I can think of so far is to return some sort of magical variable that will delegate assignment to the fake glob, turning off its fake flag temporarily during the assignment. So we will be back to hacks upon hacks, but at least this hack will solve *all* the bugs above, allowing two existing hacks in blead to be removed. This will change the meaning of \*$glob_copy, making it similar to \*$io. I think that’s a good thing. Flags: Site configuration information for perl 5.13.4: Configured by sprout at Sun Sep 5 17:19:23 PDT 2010. Summary of my perl5 (revision 5 version 13 subversion 4 patch v5.13.4-144-gf1dcae2) configuration: Locally applied patches: @INC for perl 5.13.4: Environment for perl 5.13.4: |
From @cpansproutOn Sep 12, 2010, at 12:28 PM, Father Chrysostomos wrote:
I tried just making it return a new glob if what it would return is flagged FAKE (unless it’s in the symtab, in which case it is reified), to see what would break. t/io/defout.t fails, because select() is no longer able to make PL_defoutgv point to something other than a GV. (I would consider that a bug fix.) In that case, I don’t know that it’s possible to set PL_defoutgv to a non-GV any more, so is that test file still needed? Another thing this changes is the *$foo_copy = $bar case, mentioned in the original bug report. The magical glob I suggested would complicate things too much and cause even more buggy edge cases, as the fake glob that the new glob is delegating to may not always be a glob. Since assignment to glob copies has been so buggy and unreliable in the past, this change may be acceptable as it is. But it may be better to make *{} on the left-hand side of an assignment special, so it has the current (blead) behaviour. (That is, it returns its argument if it’s a glob, fake on not, and = turns of the fakeness temporarily during assignment.) Does anyone else have any opinions? |
From @cpansproutFrom: Father Chrysostomos <sprout@cpan.org> [perl #77810] Scalars vs globs Stop *{} from returning globs with the SVf_FAKE flag on. Inline Patchdiff -up blead/op.c blead-77810-rv2gv/op.c
--- blead/op.c 2010-09-13 08:50:10.000000000 -0700
+++ blead-77810-rv2gv/op.c 2010-09-16 17:57:10.000000000 -0700
@@ -7189,6 +7189,8 @@ Perl_ck_rvconst(pTHX_ register OP *o)
#endif
kid->op_private = 0;
kid->op_ppaddr = PL_ppaddr[OP_GV];
+ /* FAKE globs in the symbol table cause weird bugs (#77810) */
+ SvFAKE_off(gv);
}
}
return o;
diff -up blead/pp.c blead-77810-rv2gv/pp.c
--- blead/pp.c 2010-09-06 08:29:10.000000000 -0700
+++ blead-77810-rv2gv/pp.c 2010-09-16 18:27:01.000000000 -0700
@@ -213,11 +213,19 @@ PP(pp_rv2gv)
}
sv = MUTABLE_SV(gv_fetchsv(sv, GV_ADD, SVt_PVGV));
}
+ /* FAKE globs in the symbol table cause weird bugs (#77810) */
+ if (sv) SvFAKE_off(sv);
}
}
if (PL_op->op_private & OPpLVAL_INTRO)
save_gp(MUTABLE_GV(sv), !(PL_op->op_flags & OPf_SPECIAL));
- SETs(sv);
+ if (sv && SvFAKE(sv)) {
+ SV *newsv = sv_newmortal();
+ sv_setsv(newsv, sv);
+ SvFAKE_off(newsv);
+ SETs(newsv);
+ }
+ else SETs(sv);
RETURN;
}
diff -rup blead/t/op/gv.t blead-77810-rv2gv/t/op/gv.t
--- blead/t/op/gv.t 2010-09-07 20:17:10.000000000 -0700
+++ blead-77810-rv2gv/t/op/gv.t 2010-09-16 18:07:26.000000000 -0700
@@ -12,7 +12,7 @@ BEGIN {
use warnings;
require './test.pl';
-plan( tests => 194 );
+plan( tests => 199 );
# type coersion on assignment
$foo = 'foo';
@@ -32,6 +32,34 @@ is(ref(\$foo), 'GLOB');
is($foo, '*main::bar');
is(ref(\$foo), 'GLOB');
+{
+ no warnings;
+ ${\*$foo} = undef;
+ is(ref(\$foo), 'GLOB', 'no type coersion when assigning to *{} retval');
+ $::{phake} = *bar;
+ is(
+ \$::{phake}, \*{"phake"},
+ 'symbolic *{} returns symtab entry when FAKE'
+ );
+ ${\*{"phake"}} = undef;
+ is(
+ ref(\$::{phake}), 'GLOB',
+ 'no type coersion when assigning to retval of symbolic *{}'
+ );
+ $::{phaque} = *bar;
+ eval '
+ is(
+ \$::{phaque}, \*phaque,
+ "compile-time *{} returns symtab entry when FAKE"
+ );
+ ${\*phaque} = undef;
+ ';
+ is(
+ ref(\$::{phaque}), 'GLOB',
+ 'no type coersion when assigning to retval of compile-time *{}'
+ );
+}
+
# type coersion on substitutions that match
$a = *main::foo;
$b = $a; |
From @cpansproutOn Sep 27, 2010, at 1:18 PM, Slaven Rezic wrote:
Thank you. That’s very good news. I’m going to forward this to the RT ticket. |
From @cpansproutOn Sun Sep 19 12:07:29 2010, sprout wrote:
And I’ve just applied it as 2acc331. If this proves unacceptable for 5.14 and is reverted, it might also be a |
|
@cpansprout - Status changed from 'new' to 'open' |
|
@cpansprout - Status changed from 'open' to 'resolved' |
From @jbenjoreOn Sun, Oct 24, 2010 at 4:22 PM, Father Chrysostomos via RT
This broke Data::Dump::Streamer Josh |
From @jbenjoreOn Sun, Jan 30, 2011 at 9:03 PM, Joshua ben Jore <twists@gmail.com> wrote:
Have worked around this in DDS-2.26. The key here is keep the glob Incorrect $Bobj = B::svref_2object(\*$name); Correct my $glob_ref = \*$name; Is this a new bug in perl? Josh |
From @cpansproutOn Mon Jan 31 19:18:44 2011, jjore wrote:
(https://rt.cpan.org/Ticket/Display.html?id=65272&results=7df022c760a8288f87e03aa553e241ec).
This was an intentional incompatible change that made *{$glob_copy} It’s the glob returned by *{} that is being freed, not the reference to The variable $lhs in DDS gets a glob assigned to it at some point (I B::svref_2object(ref \$lhs eq 'GLOB' ? \$lhs : \*$lhs) is another, |
From @jbenjoreOn Sun, Feb 6, 2011 at 1:19 PM, Father Chrysostomos via RT
Thanks for the info and clarification that this is merely an internal Jos |
Migrated from rt.perl.org#77810 (status was 'resolved')
Searchable as RT77810$
The text was updated successfully, but these errors were encountered: