dankogai / p5-encode

Encode - character encodings (for Perl 5.8 or better)
https://metacpan.org/release/Encode
37 stars 51 forks source link

Introduce new Encode check flag Encode::ONLY_PRAGMA_WARNINGS #134

Closed pali closed 6 years ago

pali commented 6 years ago

Introduce new Encode check flag Encode::ONLY_PRAGMA_WARNINGS which would tell Encode that it should report only those warnings which are currently enabled by pragma warnings. When Encode::ONLY_PRAGMA_WARNINGS is not set then Encode would report all warnings. The whole flag Encode::ONLY_PRAGMA_WARNINGS would have no effect when flag Encode::ENCODE_WARN_ON_ERR is not set.

UTF-8 and UTF-16 Encode modules are fixed to respect this new Encode::ONLY_PRAGMA_WARNINGS check flag and behave according to its setting.

This change fixes mess in Encode warnings. For more details see Perl RT bug: https://rt.perl.org/Public/Bug/Display.html?id=131683

Please review changes. CC @khwilliamson

dankogai commented 6 years ago

Thank you!

miyagawa commented 5 years ago

This change in Encode-2.99 seems to break the following code (extracted and reproduced from Plack-Middleware-Debug code):

use strict;
use Encode;
use Test::More;

package String;
use overload
  '""' => sub { ${$_[0]} }, fallback => 1;

sub new {
    my($class, $str) = @_;
    bless \$str, $class;
}

package main;

diag "Encode.pm $Encode::VERSION";
my $content = String->new("--\x{30c6}--");
my $text = Encode::encode('latin1', $content, Encode::FB_XMLCREF);
is $text, "--テ--";

done_testing;

with Encode-2.98:

test.t .. # Encode.pm 2.98
test.t .. ok   
All tests successful.
Files=1, Tests=1,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.02 cusr  0.01 csys =  0.04 CPU)
Result: PASS

with Encode-2.99:

test.t .. # Encode.pm 2.99
test.t .. 1/? 
#   Failed test at test.t line 19.
#          got: '--テ'
#     expected: '--テ--'
# Looks like you failed 1 test of 1.
test.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests 

Test Summary Report
-------------------
test.t (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=1, Tests=1,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.03 cusr  0.00 csys =  0.04 CPU)
Result: FAIL
pali commented 5 years ago

Hi! @miyagawa This change did not break it as it was broken also before. This is classic dangling pointer problem, you just did not hit it. I prepared fix in new pull request https://github.com/dankogai/p5-encode/pull/137 Important change is on line 313 as SvPVX(src) may contain incorrect pointer for scalars like objects with overloaded operators or for magic scalars...