Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of uninitialized value $i in pattern match (m//) #138

Open
XSven opened this issue Apr 5, 2023 · 7 comments
Open

Use of uninitialized value $i in pattern match (m//) #138

XSven opened this issue Apr 5, 2023 · 7 comments
Labels
bug implemented Implemented in repo, but not released yet

Comments

@XSven
Copy link

XSven commented Apr 5, 2023

Type-Tiny: 2.002001

This

use Eval::Closure   qw( eval_closure );
use Types::Standard qw( ArrayRef StrMatch );

my $KeyList = Type::Tiny->new(
  name   => 'KeyList',
  parent => ArrayRef [ StrMatch [ qr/\A [0-9A-Z]{3} \z/x ], 1 ]
);

my $assertion = eval_closure( source => 'sub { ' . $KeyList->inline_assert( '$_[0]' ) . '}' );

$assertion->( [ 'ABC', undef, 'DEF' ] );

defines a KeyList type constraint that is applied to a list with 3 elements. The second element is undef. If I set the environment variable EVAL_CLOSURE_PRINT_SOURCE to true the output is

package Eval::Closure::Sandbox_1;
sub {
  sub {
    do {
      no warnings "void";

      package Type::Tiny;
      (
        do {

          package Type::Tiny;
          ( ref( $_[ 0 ] ) eq 'ARRAY' ) and @{ $_[ 0 ] } >= 1 and do {
            my $ok = 1;
            for my $i ( @{ $_[ 0 ] } ) {
              ( $ok = 0, last ) unless do {

                package Type::Tiny;
                !ref( $i )
                  and !!( $i =~ $Types::Standard::StrMatch::expressions{ "Regexp|(?^x:\\A [0-9A-Z]{3} \\z)" } );
              }
            };
            $ok;
          }
        }
      ) or Type::Tiny::_failed_check( 42, "KeyList", $_[ 0 ], );
      $_[ 0 ];
    };
  }
  }

Use of uninitialized value $i in pattern match (m//) at (eval 129) line 3.
Reference ["ABC",undef,"DEF"] did not pass type constraint "KeyList" at strmatch.pl line 15
    "KeyList" is a subtype of "ArrayRef[StrMatch[(?^x:\A [0-9A-Z]{3} \z)],1]"
    Reference ["ABC",undef,"DEF"] did not pass type constraint "ArrayRef[StrMatch[(?^x:\A [0-9A-Z]{3} \z)],1]"
    "ArrayRef[StrMatch[(?^x:\A [0-9A-Z]{3} \z)],1]" constrains each value in the array with "StrMatch[(?^x:\A [0-9A-Z]{3} \z)]"
    "StrMatch[(?^x:\A [0-9A-Z]{3} \z)]" is a subtype of "StrMatch"
    "StrMatch" is a subtype of "Str"
    "Str" is a subtype of "Value"
    "Value" is a subtype of "Defined"
    Undef did not pass type constraint "Defined" (in $_->[1])
    "Defined" is defined as: (defined($_))

Can this warning

Use of uninitialized value $i in pattern match (m//) at (eval 124) line 3.

be avoided?

@tobyink
Copy link
Owner

tobyink commented Apr 5, 2023

Hmmm, interesting. StrMatch is supposed to be a child of Str, which does do a definedness check. It looks like StrMatch's inlining code is not being strict as its parent type, so isn't checking definedness. That's definitely a bug.

A workaround would be something like:

ArrayRef[
  ( Defined ) & ( StrMatch[ qr/\A [0-9A-Z]{3} \z/x ] ),
  1,
]

I'll get this fixed in 2.4.1 though.

@tobyink tobyink added the bug label Apr 5, 2023
@tobyink
Copy link
Owner

tobyink commented Apr 12, 2023

I actually can't reproduce this error, as eval_closure normally compiles code without use warnings. However, I still think StrMatch ought to be checking definedness, so I'll add that check.

@tobyink tobyink added the implemented Implemented in repo, but not released yet label Apr 12, 2023
@tobyink
Copy link
Owner

tobyink commented Apr 12, 2023

Ah, I see. You're using Eval::Closure, which does switch on warnings. Eval::TypeTiny does not.

@XSven
Copy link
Author

XSven commented Apr 13, 2023

I have used Params::ValidationCompiler that depends ot Eval::Closure. Now I have switched to Type::Params. I don't understand your decision not to turn on warnings in Eval::TypeTiny. How should we have detected such an issue without warnings being raised?

@tobyink
Copy link
Owner

tobyink commented Apr 13, 2023

These two lines are exactly equivalent in functionality:

                    !ref( $foo ) and $foo =~ m/.../
defined( $foo ) and !ref( $foo ) and $foo =~ m/.../

The only difference is that the first one will raise a warning if warnings are enabled and $foo is undef. They'll always give exactly the same boolean result.

How should we have detected such an issue without warnings being raised?

Skipping defined( $foo ) is only an issue because warnings are being raised.

@XSven
Copy link
Author

XSven commented Apr 13, 2023

I have understood this before. I like warnings and I treat them as issues. This is a matter of taste of course. I am very much
appreciating your effort. I have triggered an initiative in the company that I am working for to sponsor your work. Let's see. You may close this issue unless you need it to track the changes included in the next Type::Tiny release.

@tobyink
Copy link
Owner

tobyink commented Apr 13, 2023

Oh yeah, in normal code, I'd keep the warnings on for sure and would check definedness. Inline-generated code, I tend to value performance over maintainability and readability though.

I'll keep the issue open until the next release. Just adds a bit of visibility in case somebody else tried to report the same issue. (Unlikely, but can happen.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug implemented Implemented in repo, but not released yet
Projects
None yet
Development

No branches or pull requests

2 participants