readonly icon indicating copy to clipboard operation
readonly copied to clipboard

Regression: "Not enough arguments for Readonly" in v1.500.0

Open rohanpm opened this issue 11 years ago • 4 comments

Certain Readonly constructs which previously worked fine now fail with "Not enough arguments for Readonly" as of v1.500.0 - as in this testcase:

$ perl -MReadonly -E 'Readonly my $X => ("foo" =~ /bar/);'
Not enough arguments for Readonly at -e line 1.

Not sure what's going on there.

The non-contrived example which I actually hit is that my module had a line like this:

Readonly my $WINDOWS => ($OSNAME =~ m{win32}i);

...and it suddenly broke when Readonly was upgraded.

rohanpm avatar Jun 27 '14 06:06 rohanpm

This might also be why t/23-readonly.t in Params-Validate 1.11 fails with the new Readonly:

use strict;
use warnings;

use Test::Requires {
    Readonly       => '1.03',
    'Scalar::Util' => '1.20',
};

use Params::Validate qw(validate validate_pos SCALAR);
use Test::More;

{
    Readonly my $spec => { foo => 1 };
    my @p = ( foo => 'hello' );

    eval { validate( @p, $spec ) };
    is( $@, q{}, 'validate() call succeeded with Readonly spec hashref' );
}

{
    Readonly my $spec => { type => SCALAR };
    my @p = 'hello';

    eval { validate_pos( @p, $spec ) };
    is( $@, q{}, 'validate_pos() call succeeded with Readonly spec hashref' );
}

{
    Readonly my %spec => ( foo => { type => SCALAR } );
    my @p = ( foo => 'hello' );

    eval { validate( @p, \%spec ) };
    is( $@, q{}, 'validate() call succeeded with Readonly spec hash' );
}

done_testing();

Result:

#   Failed test 'validate_pos() call succeeded with Readonly spec hashref'
#   at t/23-readonly.t line 25.
#          got: 'Parameter #1 ("hello") has a type specification which is not a number. It is undef.
#  Use the constants exported by Params::Validate to declare types. at t/23-readonly.t line 24.
#       eval {...} called at t/23-readonly.t line 24
# '
#     expected: ''
# Looks like you failed 1 test of 3.
t/23-readonly.t .........................
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

https://rt.cpan.org/Public/Bug/Display.html?id=96758

pghmcfc avatar Jun 27 '14 11:06 pghmcfc

Sigh. Reverting and documenting.

Here's what went wrong: The prototype for Readonly::Readonly(...) is made to accept just about everything (simple scalars, hashref, etc.) so it doesn't care about undefined (not verbosely undef values) but the verbose forms (Readonly::Scalar(), et al) throw fatal errors during compile thanks to their prototypes and gag on them if passed at runtime.

Readonly::Scalar my $blah => undef; # perfectly fine
Readonly::Scalar my $blah; # fatal if caught in compile
Readonly my $blah; # slips on through; you get an undef readonly value

The documentation never mentioned this really obvious difference in the right places for some so when I received a report asking about closing what they saw as a bug in Readonly::Readonly(), I went for API consistency and made it croak(). That was stupid.

Aside: too many people have been using undocumented functionality for me to correct anything meaningful in the API without breaking a lot of code. I need to print a banner of that and hang it somewhere to remind me. :) Anyway, yeah, reverting and documenting. Next version will be on PAUSE in a few hours. I'll close this with the commit later.

Edit: Oh, and thanks for the report! :) ...more annoyed with the API than I am with users. With the sighing and all it may come off a little gruff. Really, though, thanks for the report and for using Readonly in the first place.

sanko avatar Jun 27 '14 11:06 sanko

@pghmcfc that seems to be something else entirely. Works fine over the old tie() API (if I change the test to use Readonly::Scalar1 ...) but not the XS/built-in version (uses SvREADONLY). I'll need to dig to see where it's going wrong.

sanko avatar Jun 27 '14 12:06 sanko

@pghmcfc Found it. Weird that a smoker with Readonly::XS installed didn't shake this one out long ago. Should be resolved in 1.60.

sanko avatar Jun 27 '14 15:06 sanko