bug: passing \undef to alternative as value result in undefined behaviour

[Ruslan Zakirov]

Hi,

I know that at some point ignorable products was optimized and resulted in undefined (=== random) values in actions, but hell let me do what I mean.

Doc says pass reference to value and I'm passing it reference to undef.

It worked just fine at some point but then it was broken.

Responsible change is 

dae6a5697dec6a5bb039418e3329db615e535dec and the following part in particular:

-        push @{$token_values}, ${$value_ref};

+ my $value = ${$value_ref};

+ last SET_VALUE_IX if not defined $value;

+        push @{$token_values}, $value;

I don't see how it is related to whole change.

Also, I see that "# Position 1 is reserved for undef" in $token_values then this probably just a bug and the following should be applied:

diff --git a/cpan/lib/Marpa/R2/Recognizer.pm b/cpan/lib/Marpa/R2/Recognizer.pm

index d700590..219e839 100644

--- a/cpan/lib/Marpa/R2/Recognizer.pm

+++ b/cpan/lib/Marpa/R2/Recognizer.pm

@@ -669,10 +669,9 @@ sub Marpa::R2::Recognizer::alternative {

         {

             Marpa::R2::exception('alternative(): value must be undef or ref');

         } ## end if ( $ref_type ne 'SCALAR' and $ref_type ne 'REF' and...)

+        last SET_VALUE_IX if not defined $$value_ref;

         $value_ix = scalar @{$token_values};

-        my $value = ${$value_ref};

-        last SET_VALUE_IX if not defined $value;

-        push @{$token_values}, $value;

+        push @{$token_values}, $$value_ref;

     } ## end SET_VALUE_IX:

     $length //= 1;

--
Best regards, Ruslan.

[Jeffrey Kegler]

Things do look problematic here.

Could you create a small example duplicating the failure?  An advantage of these is that it clarifies for me exactly what you expect to happen.

A complication here is that in addition to Perl undef's, I introduced "whatever" variables to Marpa.  These are allowed to have any value -- presumably they'd be for cases where you ignore the value.  That is, an 'undefined' value must be undef, but a 'whatever' value can be undef one time, 42 the next, and "hello" the next.

"Whatever" values were intended to save stack operations.  Marpa's NAIF did these via callback, so every stack op save was a big win.  When a whatever value needs to be put on the stack, it's a no-op, whereas when an Perl undef needs to be put on the stack, it's a callback -- a vast difference in resource.

With the SLIF, I implement callbacks more cheaply, and future changes may reduce this cost further.  This means the time and trouble to track whatever values is probably not worth it.  "Whatever" values may be deprecated in Libmarpa.  I think I kept them out of the Marpa::R2 documentation, so this deprecation would only be relevant at the Libmarpa and THIF levels.

-- jeffrey

--
You received this message because you are subscribed to the Google Groups "marpa parser" group.
To unsubscribe from this group and stop receiving emails from it, send an email to marpa-parser...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Ruslan Zakirov]

use strict; use warnings; use 5.010;

use Marpa::R2;

use Marpa::R2::Grammar;

use MarpaX::Repa::Actions;

my $grammar = Marpa::R2::Grammar->new( {

    start   => 'start',

    actions => 'main',

    default_action => 'dummy',

    rules   => [

        [ start => [qw/x y/] ], 

    ], 

} ); 

$grammar->precompute;

my $rec = Marpa::R2::Recognizer->new( { grammar => $grammar } ); 

$rec->alternative('x',\undef, 1);

$rec->earleme_complete;

$rec->alternative('y',\"some", 1);

$rec->earleme_complete;

use Data::Dumper;

say Dumper $rec->value;

sub dummy {

    shift;

    return [@_];

}

--
Best regards, Ruslan.

[Ruslan Zakirov]

Hi,

As far as I can see in new code R2::Recognizer never uses whatever anymore (IX == 0) and from changelog suspect that whatever was left for users of Thin interface. Earlier mentioned patch makes sure that IX == 1 is used instead and produces undef. I'm not sure if it's optimized on lower level or not (calls callback or not), but for sure it's way more usable.

Two thoughts:

1) If whole "whatever" thing is about optimization and not calling callbacks then may be libmarpa can reserve an ix for constant value that can be set as an option from outside and avoid calling callback.

2) I would find it helpful to have "skip" mode instead of whatever. Basicly $rec->read('x') would advance recognizer, but action for the rule would get less arguments. $rec->read('x', undef) would result in the current behaviour with undef in the action. Anyway most of helper actions I wrote so far do @_ = grep defined, @_;

On Tue, Jan 7, 2014 at 9:52 PM, Jeffrey Kegler <jeffre...@jeffreykegler.com> wrote:

--
Best regards, Ruslan.

[Jeffrey Kegler]

OK.  I downloaded it and duplicated the problem.   The behavior is clearly both a bug and not as documented. -- jeffrey

[Jeffrey Kegler]

The fix is commit c8d296b, just pushed.

The business with "whatever" tokens is a separate issue -- involving the same logic to be sure, but a separate issue.  I'll look at that separately and next. -- jeffrey

On 01/07/2014 11:43 AM, Ruslan Zakirov wrote: