[PATCH] Include the name of the non-lvalue sub in error message #14969
Comments
From @ilmariCreated by @ilmariThis makes the cause of the error more obvious if you accidentally call E.g. when adding :lvalue to the AUTOLOAD sub in Struct::Dumb¹, it took a [1] https://metacpan.org/source/PEVANS/Struct-Dumb-0.04/lib/Struct/Dumb.pm#L159 Perl Info |
From @ilmari0001-Include-the-name-of-the-non-lvalue-sub-in-error-mess.patchFrom 30e379a544579363811d8b35c4fe4b5527859188 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 6 Oct 2015 23:13:31 +0100
Subject: [PATCH] Include the name of the non-lvalue sub in error message
This makes the cause of the error more obvious if you accidentally call
a non-lvalue sub in the final position of an lvalue one.
---
op.c | 10 ++++++++++
pod/perldiag.pod | 2 +-
pp_hot.c | 6 ++++--
t/op/sub_lval.t | 16 ++++++++--------
4 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/op.c b/op.c
index 399cf30..2d3fcd8 100644
--- a/op.c
+++ b/op.c
@@ -2792,6 +2792,7 @@ Perl_op_lvalue_flags(pTHX_ OP *o, I32 type, U32 flags)
OP *kid = cUNOPo->op_first;
CV *cv;
GV *gv;
+ SV *namesv;
if (kid->op_type != OP_PUSHMARK) {
if (kid->op_type != OP_NULL || kid->op_targ != OP_LIST)
@@ -2829,6 +2830,15 @@ Perl_op_lvalue_flags(pTHX_ OP *o, I32 type, U32 flags)
break;
if (CvLVALUE(cv))
break;
+ if (flags & OP_LVALUE_NO_CROAK)
+ return NULL;
+
+ namesv = cv_name(cv, NULL, 0);
+ yyerror_pv(Perl_form(aTHX_ "Can't modify non-lvalue "
+ "subroutine call of &%"SVf" in %s",
+ SVfARG(namesv), PL_op_desc[type]),
+ SvUTF8(namesv));
+ return o;
}
}
/* FALLTHROUGH */
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index db21861..d40b093 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -1108,7 +1108,7 @@ to change it, such as with an auto-increment.
(P) The internal routine that does assignment to a substr() was handed
a NULL.
-=item Can't modify non-lvalue subroutine call
+=item Can't modify non-lvalue subroutine call of &%s
(F) Subroutines meant to be used in lvalue context should be declared as
such. See L<perlsub/"Lvalue subroutines">.
diff --git a/pp_hot.c b/pp_hot.c
index 66e8b9d..9f34ae6 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3426,7 +3426,8 @@ PP(pp_entersub)
SAVETMPS;
if (UNLIKELY((cx->blk_u16 & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO &&
!CvLVALUE(cv)))
- DIE(aTHX_ "Can't modify non-lvalue subroutine call");
+ DIE(aTHX_ "Can't modify non-lvalue subroutine call of &%"SVf,
+ SVfARG(cv_name(cv, NULL, 0)));
/* warning must come *after* we fully set up the context
* stuff so that __WARN__ handlers can safely dounwind()
* if they want to
@@ -3447,7 +3448,8 @@ PP(pp_entersub)
& PUSHSUB_GET_LVALUE_MASK(Perl_is_lvalue_sub)
) & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO &&
!CvLVALUE(cv)))
- DIE(aTHX_ "Can't modify non-lvalue subroutine call");
+ DIE(aTHX_ "Can't modify non-lvalue subroutine call of &%"SVf,
+ SVfARG(cv_name(cv, NULL, 0)));
if (UNLIKELY(!hasargs && GvAV(PL_defgv))) {
/* Need to copy @_ to stack. Alternative may be to
diff --git a/t/op/sub_lval.t b/t/op/sub_lval.t
index ab9faac..f70e6fe 100644
--- a/t/op/sub_lval.t
+++ b/t/op/sub_lval.t
@@ -169,7 +169,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call in scalar assignment/);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::nolv in scalar assignment/);
$_ = '';
@@ -178,7 +178,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call in scalar assignment/);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::nolv in scalar assignment/);
$_ = '';
@@ -187,7 +187,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call in scalar assignment/);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::nolv in scalar assignment/);
$x0 = $x1 = $_ = undef;
$nolv = \&nolv;
@@ -358,7 +358,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call at /);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::xxx at /);
$_ = undef;
eval <<'EOE' or $_ = $@;
@@ -366,7 +366,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call at /);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::xxx at /);
sub yyy () { 'yyy' } # Const, not lvalue
@@ -823,7 +823,7 @@ foo = 3;
----
lvalue attribute ignored after the subroutine has been defined at - line 4.
lvalue attribute ignored after the subroutine has been defined at - line 6.
-Can't modify non-lvalue subroutine call in scalar assignment at - line 7, near "3;"
+Can't modify non-lvalue subroutine call of &main::foo in scalar assignment at - line 7, near "3;"
Execution of - aborted due to compilation errors.
====
}
@@ -979,7 +979,7 @@ package _102486 {
'sub:lvalue{&$x}->() does not die for non-lvalue inner sub call';
::is $called, 1, 'The &$x actually called the sub';
eval { +sub :lvalue { &$x }->() = 3 };
- ::like $@, qr/^Can't modify non-lvalue subroutine call at /,
+ ::like $@, qr/^Can't modify non-lvalue subroutine call of &_102486::nonlv at /,
'sub:lvalue{&$x}->() dies in true lvalue context';
}
@@ -1008,7 +1008,7 @@ for (sub : lvalue { "$x" }->()) {
# [perl #117947] XSUBs should not be treated as lvalues at run time
eval { &{\&utf8::is_utf8}("") = 3 };
-like $@, qr/^Can't modify non-lvalue subroutine call at /,
+like $@, qr/^Can't modify non-lvalue subroutine call of &utf8::is_utf8 at /,
'XSUB not seen at compile time dies in lvalue context';
# [perl #119797] else implicitly returning value
--
2.5.1
|
From @ilmariDagfinn Ilmari Mannsåker (via RT) <perlbug-followup@perl.org> writes:
Here's an updated patch which also removes the now-redundant OP_ENTERSUB |
From @ilmari0001-Include-the-name-of-the-non-lvalue-sub-in-error-mess.patchFrom 2c727a4896e53e09b4c5ac00b386833b157bd55d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 6 Oct 2015 23:13:31 +0100
Subject: [PATCH] Include the name of the non-lvalue sub in error message
This makes the cause of the error more obvious if you accidentally call
a non-lvalue sub in the final position of an lvalue one.
---
op.c | 14 +++++++++++---
pod/perldiag.pod | 2 +-
pp_hot.c | 6 ++++--
t/op/sub_lval.t | 16 ++++++++--------
4 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/op.c b/op.c
index 399cf30..c0c321b 100644
--- a/op.c
+++ b/op.c
@@ -2792,6 +2792,7 @@ Perl_op_lvalue_flags(pTHX_ OP *o, I32 type, U32 flags)
OP *kid = cUNOPo->op_first;
CV *cv;
GV *gv;
+ SV *namesv;
if (kid->op_type != OP_PUSHMARK) {
if (kid->op_type != OP_NULL || kid->op_targ != OP_LIST)
@@ -2829,6 +2830,15 @@ Perl_op_lvalue_flags(pTHX_ OP *o, I32 type, U32 flags)
break;
if (CvLVALUE(cv))
break;
+ if (flags & OP_LVALUE_NO_CROAK)
+ return NULL;
+
+ namesv = cv_name(cv, NULL, 0);
+ yyerror_pv(Perl_form(aTHX_ "Can't modify non-lvalue "
+ "subroutine call of &%"SVf" in %s",
+ SVfARG(namesv), PL_op_desc[type]),
+ SvUTF8(namesv));
+ return o;
}
}
/* FALLTHROUGH */
@@ -2842,9 +2852,7 @@ Perl_op_lvalue_flags(pTHX_ OP *o, I32 type, U32 flags)
yyerror(Perl_form(aTHX_ "Can't modify %s in %s",
(o->op_type == OP_NULL && (o->op_flags & OPf_SPECIAL)
? "do block"
- : (o->op_type == OP_ENTERSUB
- ? "non-lvalue subroutine call"
- : OP_DESC(o))),
+ : OP_DESC(o)),
type ? PL_op_desc[type] : "local"));
return o;
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index db21861..d40b093 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -1108,7 +1108,7 @@ to change it, such as with an auto-increment.
(P) The internal routine that does assignment to a substr() was handed
a NULL.
-=item Can't modify non-lvalue subroutine call
+=item Can't modify non-lvalue subroutine call of &%s
(F) Subroutines meant to be used in lvalue context should be declared as
such. See L<perlsub/"Lvalue subroutines">.
diff --git a/pp_hot.c b/pp_hot.c
index 66e8b9d..9f34ae6 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3426,7 +3426,8 @@ PP(pp_entersub)
SAVETMPS;
if (UNLIKELY((cx->blk_u16 & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO &&
!CvLVALUE(cv)))
- DIE(aTHX_ "Can't modify non-lvalue subroutine call");
+ DIE(aTHX_ "Can't modify non-lvalue subroutine call of &%"SVf,
+ SVfARG(cv_name(cv, NULL, 0)));
/* warning must come *after* we fully set up the context
* stuff so that __WARN__ handlers can safely dounwind()
* if they want to
@@ -3447,7 +3448,8 @@ PP(pp_entersub)
& PUSHSUB_GET_LVALUE_MASK(Perl_is_lvalue_sub)
) & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO &&
!CvLVALUE(cv)))
- DIE(aTHX_ "Can't modify non-lvalue subroutine call");
+ DIE(aTHX_ "Can't modify non-lvalue subroutine call of &%"SVf,
+ SVfARG(cv_name(cv, NULL, 0)));
if (UNLIKELY(!hasargs && GvAV(PL_defgv))) {
/* Need to copy @_ to stack. Alternative may be to
diff --git a/t/op/sub_lval.t b/t/op/sub_lval.t
index ab9faac..f70e6fe 100644
--- a/t/op/sub_lval.t
+++ b/t/op/sub_lval.t
@@ -169,7 +169,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call in scalar assignment/);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::nolv in scalar assignment/);
$_ = '';
@@ -178,7 +178,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call in scalar assignment/);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::nolv in scalar assignment/);
$_ = '';
@@ -187,7 +187,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call in scalar assignment/);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::nolv in scalar assignment/);
$x0 = $x1 = $_ = undef;
$nolv = \&nolv;
@@ -358,7 +358,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call at /);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::xxx at /);
$_ = undef;
eval <<'EOE' or $_ = $@;
@@ -366,7 +366,7 @@ eval <<'EOE' or $_ = $@;
1;
EOE
-like($_, qr/Can\'t modify non-lvalue subroutine call at /);
+like($_, qr/Can\'t modify non-lvalue subroutine call of &main::xxx at /);
sub yyy () { 'yyy' } # Const, not lvalue
@@ -823,7 +823,7 @@ foo = 3;
----
lvalue attribute ignored after the subroutine has been defined at - line 4.
lvalue attribute ignored after the subroutine has been defined at - line 6.
-Can't modify non-lvalue subroutine call in scalar assignment at - line 7, near "3;"
+Can't modify non-lvalue subroutine call of &main::foo in scalar assignment at - line 7, near "3;"
Execution of - aborted due to compilation errors.
====
}
@@ -979,7 +979,7 @@ package _102486 {
'sub:lvalue{&$x}->() does not die for non-lvalue inner sub call';
::is $called, 1, 'The &$x actually called the sub';
eval { +sub :lvalue { &$x }->() = 3 };
- ::like $@, qr/^Can't modify non-lvalue subroutine call at /,
+ ::like $@, qr/^Can't modify non-lvalue subroutine call of &_102486::nonlv at /,
'sub:lvalue{&$x}->() dies in true lvalue context';
}
@@ -1008,7 +1008,7 @@ for (sub : lvalue { "$x" }->()) {
# [perl #117947] XSUBs should not be treated as lvalues at run time
eval { &{\&utf8::is_utf8}("") = 3 };
-like $@, qr/^Can't modify non-lvalue subroutine call at /,
+like $@, qr/^Can't modify non-lvalue subroutine call of &utf8::is_utf8 at /,
'XSUB not seen at compile time dies in lvalue context';
# [perl #119797] else implicitly returning value
--
2.5.1
|
From @ilmari-- |
From @rurbanOn Oct 7, 2015, at 1:02 AM, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
Suggestion: See e.g. my cperl CV_NAME_NOMAIN patches Reini Urban
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @ilmariReini Urban <reini.urban@gmail.com> writes:
That is a orthogonal issue: we report it in other contexts, so this one $ perl -we 'foo()' -- |
From @rurban
Sure. Not talking about your patch which is fine. I’m talking about on top of your patch, because it reminded me on And you touched almost all those places already here. |
From @ilmariReini Urban <reini.urban@gmail.com> writes:
Please start a new thread for that, or even better, file an RT with just
I touched only one of the several error mesages that report the sub ilmari@garkbit:~/src/perl$ ag --cc '(?<!^)\bcv_name\(' | wc -l -- |
From @tonycozOn Tue Oct 06 16:03:16 2015, ilmari wrote:
Thanks, applied as 0f94828. Tony |
|
@tonycoz - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#126281 (status was 'resolved')
Searchable as RT126281$
The text was updated successfully, but these errors were encountered: