Suppress 'subroutine redefined' warning

[jkeenan]

... which was triggered by introduction of 'use v5.42;'

Fixes: GH #24774

---

• This set of changes does not require a perldelta entry.

[Leont]

I don't like this solution. It doesn't look correct, if it's not reasonable to warn about redefinition here then the reason for that should be explained.

AFAICT, the problem is that t/overload.t reuses package Oscalar. I think that a better solution should be found there instead.

[jkeenan]

I don't like this solution. It doesn't look correct, if it's not reasonable to warn about redefinition here then the reason for that should be explained.

AFAICT, the problem is that t/overload.t reuses package Oscalar. I think that a better solution should be found there instead.

Okay, but lib/overload.t is a mess.

• It doesn't use strict;.
• It doesn't use warnings;.
• It has a large block of tests that need to run in taint mode.

$ grep -nE 'taint|\b-T\b' lib/overload.t
15:my $no_taint_support = exists($Config::Config{taint_support})
16:                     && !$Config::Config{taint_support};
18:my %skip_fetch_count_when_no_taint = (
27:sub is_if_taint_supported {
29:    if ($expected && $no_taint_support) {
30:        return skip("your perl was built without taint support");
78:use Scalar::Util qw(tainted);
1693:# We also check that return values are correctly tainted.
1726:    #  * Under the taint version of the tests,  whether we expect
1727:    #       the result to be tainted (for example comparison ops
1728:    #       like '==' don't return a tainted value, even if their
1909:	# eval should do tie, overload on its arg before checking taint */
1987:	    $exp_counts, $exp_taint) = @$test;
1989:	my $tainted_val;
1991:	    # create tainted version of $val (unless its a ref)
1995:	    $tainted_val = ref($val1) ? $val :
1998:	$tie_val = $tainted_val;
2002:	    my $plain = $tainted_val;
2007:		is_if_taint_supported(tainted($exp), $exp_taint,
2008:		    "<$plain_term> taint of expected return");
2021:		$ta[0]    = bless [ $tainted_val ], $ov_pkg;
2022:		my $oload = bless [ $tainted_val ], $ov_pkg;
2038:		    # find out if its tainted
2041:			is_if_taint_supported(tainted($res), $exp_taint,
2042:			    "$desc taint of result return");
2057:			if ($skip_fetch_count_when_no_taint{$desc} && $no_taint_support) {
2058:			    skip("your perl was built without taint support");

• It jumps from one package into another over 100 times.

$ grep -nE '\bpackage\b' lib/overload.t
38:package Oscalar;
72:package main;
122:eval q[ package Oscalar; use overload ('++' => sub { $ {$_[0]}++;$_[0] } ) ];
135:package Oscalar;
137:package main;
149:eval q[package Oscalar; use overload ('++' => sub { $ {$_[0]} += 2; $_[0] } ) ];
162:package Oscalar;
164:package main;
184:eval q[ package Oscalar; use overload ('=' => sub {$main::copies++; 
185:						   package Oscalar;
230:eval q[package Oscalar; use overload ('+=' => sub {$ {$_[0]} += 3*$_[1];
266:eval q[package Oscalar; 
274:eval q[package Oscalar; 
285:  package OscalarI;
296:# blessing to a package update hash
298:eval "package Oscalar; no overload '.'";
318:eval "package Oscalar; sub comple; use overload '~' => 'comple'";
343:eval "package Oscalar; sub rshft; use overload '>>' => 'rshft'";
372:  package OscalarII;
375:  eval "package OscalarI; use overload '<<' => 'lshft', '|' => 'lshft'";
458:  package symbolic;		# Primitive symbolic calculator
581:  package symbolic1;		# Primitive symbolic calculator
701:  package two_face;		# Scalars with separate string and
717:  package sorting;
729:  package iterator;
748:  package deref;
826:  package two_refs;
864:  package two_refs_o;
875:  package two_refs1;
917:  package two_refs1_o;
928:  package B;
945:  package Join;
1004:  package ov_int1;
1010:  package ov_int2;
1016:  package noov_int;
1021:  package main;
1037:  package Recurse;
1050:package Foo;
1070:package main;
1079:package utf8_o;
1093:package main;
1104:package Hderef;
1106:package Foo;
1111:package main;
1119:   package t229;
1147:    package perl31793;
1149:    package perl31793_fb;
1151:    package main;
1168:    package Numify;
1202:	package Foo493;
1224:        package QRonly;
1254:        package QRandSTR;
1273:        package QRany;
1276:        package QRself;
1296:        package STRonly;
1299:        package STRonlyFB;
1334:    package Sklorsh;
1355:    package Brap;
1359:    package main;
1379:    package Flrbbbbb;
1388:    package main;
1421:    package Shklitza;
1424:    package Ksshfwoom;
1426:    package main;
1448:    package bit;
1461:    package main;
1485:    package nomethod_false;
1488:    package nomethod_true;
1491:    package main;
1507:        package nomethod_false;
1516:        package nomethod_true;
1531:    package nomethod_not;
1534:    package main;
1551:    package kayo;
1559:    package main;
1585:    package numify_int;
1587:    package numify_self;
1589:    package numify_other;
1591:    package numify_by_fallback;
1594:    package main;
1636:    package CopyConstructorFallback;
1642:    package main;
1654:    package REPEAT;
1667:    package main;
1928:    package RT57012_OV;
1937:    package RT57012_OV_FB; # only contains fallback conversion functions
1970:    package RT57012_TIE_S;
1977:    package RT57012_TIE_A;
1983:    package main;
2073:# Test overload from the main package
2078: 'use overload from the main package'
2082:    package blessed_methods;
2094:    package CmpBase;
2104:    package NCmp;
2108:    package SCmp;
2112:    package main;
2145:    package NuMB;
2163:    package main;
2172:                    package NuMB$fall$nomethod;
2236:    package Iter1;
2238:    package main;
2253:    package Justus;
2260:    package JustUs;
2262:    package JustYou { use overload '+' => 'injustice'; }
2266:                  )g "\+" in package "JustUs" at /,
2273:    package utf8_match;
2316:package proxy {
2325:package o     { use overload '""' => sub { 'keck' };
2334:    package broken_can;
2338:    package main;
2350:    eval 'package eleventative; use overload map +($_)x2, cos=>abs=>';
2366:{ package phake;
2375:{ package ent; use overload fallback => 0, abs => sub{};
2377:  package huorn;
2379:  package ent;
2389: package pervyy;
2392: package vtoryy;
2400:# package separators in method names
2402: package mane;
2415:  package dodo;
2429:        package Splitter;
2494:        package StringWithContext;
2537:        package AddWithContext;
2685:        package OnlyFallback;
2697:        package OnlyFallbackUndef;
2710:        package OnlyFallbackTrue;
2729:package Regexp;
2731:package main;
2749:    package A_121362;
2754:    package B_121362;
2757:    package main;
2770:package refsgalore {
2787:package xstack { use overload 'x' => sub { shift . " x " . shift },
2793:package bitops {
2828:package bitops2 {
2856:package length_utf8 {
2867:    package ant;
2879:package Concat {
3054:package RT132385 {
3091:package RT132783 {
3104:package RT132827 {
3115:package RT132793 {
3132:package RT132801 {
3173:package Stringify {
3219:package RT33789 {
3239:package GH21477 {

Who wants to take on that refactoring?

[jkeenan]

AFAICT, the problem is that t/overload.t reuses package Oscalar. I think that a better solution should be found there instead.

It is at line 147 of lib/overload.t that the warning is emitted.

ok 30 - [at ../lib/overload.t line 144]
Subroutine Oscalar::(++ redefined at ../lib/overload.pm line 49.

 133 package Oscalar;
 134 $dummy=bless \$dummy;       # Now cache of method should be reloaded
 135 package main;
 136 
 137 $b=$a;
 138 $b++;
 139 
 140 is(ref $b, "Oscalar");
 141 is($a, "087");
 142 is($b, "88");
 143 is(ref $a, "Oscalar");
 144 
 145 undef $b;           # Destroying updates tables too...
 146 
 147 eval q[package Oscalar; use overload ('++' => sub { $ {$_[0]} += 2; $_[0] } ) ];

[tonycoz]

The test here is deliberately redefining the ++ overload for Oscalar to check if the internal overloading tables are properly updated, so I don't think it can be fixed purely in lib/overload.t.

So:

• do we want overload redefinitions to warn? (it seems useful in the general case, such redefinitions seem like an error)
• it currently warns with in overload.pm, rather than in the code redefining the overload
• it checks the overload.pm warning flags (it should check the caller's flags)
• it checks the core redefine warning category - should it use a separate category?

To me the ideal behaviour would be:

Warn in the location of the caller if the overload is redefined and the core redefine warning is enabled in the caller.

[tonycoz]

#24777 maybe

I learned even more about overloading internals, I'm not sure that was a good idea.