{"action":"opened","issue":{"url":"https://api.github.com/repos/jeffreykegler/Marpa--R2/issues/254","labels_url":"https://api.github.com/repos/jeffreykegler/Marpa--R2/issues/254/labels{/name}","comments_url":"https://api.github.com/repos/jeffreykegler/Marpa--R2/issues/254/comments","events_url":"https://api.github.com/repos/jeffreykegler/Marpa--R2/issues/254/events","html_url":"https://github.com/jeffreykegler/Marpa--R2/issues/254","id":72792310,"number":254,"title":"SLIF does not respect blessed per-parse arguments","user":{"login":"latk","id":1782927,"avatar_url":"https://avatars.githubusercontent.com/u/1782927?v=3","gravatar_id":"","url":"https://api.github.com/users/latk","html_url":"https://github.com/latk","followers_url":"https://api.github.com/users/latk/followers","following_url":"https://api.github.com/users/latk/following{/other_user}","gists_url":"https://api.github.com/users/latk/gists{/gist_id}","starred_url":"https://api.github.com/users/latk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/latk/subscriptions","organizations_url":"https://api.github.com/users/latk/orgs","repos_url":"https://api.github.com/users/latk/repos","events_url":"https://api.github.com/users/latk/events{/privacy}","received_events_url":"https://api.github.com/users/latk/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2015-05-03T11:24:54Z","updated_at":"2015-05-03T11:24:54Z","closed_at":null,"body":"I am not sure whether this is a bug, a documentation bug, or per-design, but it's surely annoying. This issue is a more thorough discussion of the problem mentioned in http://irclog.perlgeek.de/marpa/2015-05-02#i_10537778 and http://irclog.perlgeek.de/marpa/2015-05-03#i_10538150\r\n\r\n# The problem\r\n\r\nThe documentation for [Marpa::R2::Scanless::R::value](https://metacpan.org/pod/Marpa::R2::Scanless::R#value) indicates that the argument to value `$recce->value($arg)` will be the per-parse value for all rule evaluation closures.\r\n\r\nThe Semantics documentation instead states that if a per-parse constructor exists (i.e. a `new` sub in the semantics package), then [the argument of `$recce->value` will be used as the sole argument to the per-parse constructor](https://metacpan.org/pod/Marpa::R2::Semantics#The-per-parse-argument). This makes sense if both the value argument is not blessed, and also the per-parse constructor is a function instead of a method. But if the value argument is blessed, this leads to highly unintuitive behaviour.\r\n\r\nConsider this program, using Moo for OOP support. It accidentally works, obscuring the problem:\r\n\r\n``` perl\r\npackage MyActions;\r\nuse Moo;\r\nuse feature 'say';\r\nuse Marpa::R2;\r\n \r\nsub parse {\r\n    my ($class, $input) = @_;\r\n    my $grammar = Marpa::R2::Scanless::G->new({ source => \\q(A ::= 'a' action => do_A) });\r\n    my $recce = Marpa::R2::Scanless::R->new({ grammar => $grammar });\r\n    $recce->read(\\$input);\r\n    my $per_parse_argument = $class->new;\r\n    my $value = $recce->value($per_parse_argument);\r\n    return $$value;\r\n}\r\n \r\nsub do_A {\r\n    my ($self, $letter) = @_;\r\n    return \"read '$letter'\";\r\n}\r\n \r\nsay MyActions->parse('a');\r\n```\r\n\r\nThis resolves `MyActions` as the semantics package, and sets the per-parse argument to a `MyActions` instance – but not to the same instance we passed to the `$recce->value` method!\r\n\r\nHowever, the `new` method might not be a “default constructor”, in which case the difference can be seen:\r\n\r\n``` perl\r\npackage MyActions;\r\nuse Moo;\r\nuse feature 'say';\r\nuse Marpa::R2;\r\n \r\nhas data => (\r\n    is => 'ro',\r\n    required => 1,\r\n);\r\n \r\nsub parse {\r\n    my ($class, $input) = @_;\r\n    my $grammar = Marpa::R2::Scanless::G->new({ source => \\q(A ::= 'a' action => do_A) });\r\n    my $recce = Marpa::R2::Scanless::R->new({ grammar => $grammar });\r\n    $recce->read(\\$input);\r\n    my $per_parse_argument = $class->new(data => \"foo\");\r\n    my $value = $recce->value($per_parse_argument);\r\n    return $$value;\r\n}\r\n \r\nsub do_A {\r\n    my ($self, $letter) = @_;\r\n    return \"read '$letter'\";\r\n}\r\n \r\nsay MyActions->parse('a');\r\n\r\n```\r\n\r\nThis will now blow up with a cryptic error message:\r\n\r\n``` plain\r\n============================================================\r\n* THE MARPA SEMANTICS PRODUCED A FATAL ERROR\r\n* THIS IS WHAT MARPA WAS DOING WHEN THE PROBLEM OCCURRED:\r\nconstructing action object\r\n* THIS WAS THE FATAL ERROR MESSAGE:\r\nMissing required arguments: data at (eval 136) line 49.\r\nMarpa::R2 exception at so2.pl line 18.\r\n```\r\n\r\nThe problem is that Marpa does something like this:\r\n\r\n``` perl\r\n$semantics_package //= ref $argument_to_value // die;\r\nif (my $code = *{ $semantics_package . '::new' }{CODE}) {\r\n    $per_parse_constructor = $code ;\r\n}\r\nmy $per_parse_argument = $argument_to_value;\r\nif ($per_parse_constructor) {\r\n    my $ctor_argument = $per_parse_argument // $semantics_package;\r\n    eval { $per_parse_argument = $per_parse_constructor->($ctor_argument) };\r\n}\r\n...\r\n```\r\n\r\n(for the real code, see https://github.com/jeffreykegler/Marpa--R2/blob/491cb3b0cea473ae79978037c24455b3d73c8b4a/cpan/lib/Marpa/R2/Value.pm#L1356-L1560)\r\n\r\nThis means that if the `$argument_to_value` is a blessed object, the actual parse argument will be set to `$argument_to_value->new`! While some people implement `$instance->new` as a clone operation, this requirement is not obvious, and in fact makes it impossible to use a `new` method as commonly used in Perl:\r\n\r\n``` perl\r\nsub new {\r\n    my ($class, %args) = @_;\r\n    validate(\\%args);\r\n    return bless \\%args, $class;\r\n}\r\n```\r\n\r\nIf we want to pass arguments to a per-parse constructor with Marpa, we need to create an otherwise unusable `new` function:\r\n\r\n``` perl\r\nsub new {\r\n    my ($args) = @_;\r\n    # the first arg might be a reference or the name of the semantics package\r\n    # in that case, substitute an empty hashref\r\n    $args = {} if not ref $args;\r\n    validate($args);\r\n    return bless $args, __PACKAGE__;\r\n}\r\n```\r\n\r\nOh dear. Note that object systems such as Moo do not really support such weird constructors.\r\n\r\n# Suggested Solution\r\n\r\nIf the argument to `$recce->value` is a blessed object, do not apply the per-parse constructor to it. Instead, use that object directly and only validate that `$object->DOES($semantics_package)`, or if you really want to rule out inheritance that `ref($object) eq $semantics_package`.\r\n\r\n# Temporary Workarounds\r\n\r\n  - hide the `new` sub from Marpa when calling `$recce->value`. Note that this solution is not reentrant.\r\n\r\n    ``` perl\r\n    ...\r\n    my $per_parse_argument = MyActions->new(%args);\r\n    local *MyActions::new;\r\n    my $value = $recce->value($per_parse_argument);\r\n    ```\r\n\r\n  - don't use helpful modules like `Moo`, and instead write constructors and accessors manually – or don't use objects at all. Apparently, Marpa isn't really meant to be used in an OOP context.\r\n\r\n  - use a different package for the semantics and the per-parse object. For example, we could use a `Moo::Role`  for the semantics package, and have a `Moo` class inherit from that which is used to create the object. Since roles do not have a `new` method, there's no per-parse constructor to interfere. However, I'm surprised this actually works since the per-parse argument isn't directly blessed into the `MyActions` semantics package.\r\n\r\n    ``` perl\r\n    package MyActions;\r\n    use Moo::Role;\r\n    use feature 'say';\r\n    use Marpa::R2;\r\n     \r\n    has data => (\r\n        is => 'ro',\r\n        required => 1,\r\n    );\r\n    \r\n    {\r\n        package MyActions::Impl;\r\n        use Moo;\r\n        with 'MyActions';\r\n    }\r\n \r\n    sub parse {\r\n        my ($class, $input) = @_;\r\n        my $grammar = Marpa::R2::Scanless::G->new({ source => \\q(A ::= 'a' action => do_A) });\r\n        my $recce = Marpa::R2::Scanless::R->new({ grammar => $grammar, semantics_package => 'MyActions' });\r\n        $recce->read(\\$input);\r\n        my $per_parse_argument = $class->new(data => \"foo\");\r\n        my $value = $recce->value($per_parse_argument);\r\n        return $$value;\r\n    }\r\n     \r\n    sub do_A {\r\n        my ($self, $letter) = @_;\r\n        return \"read '$letter'\";\r\n    }\r\n     \r\n    say MyActions::Impl->parse('a');\r\n```"}}