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

“parser did not give circumfix an EXPR” after “Re-implement colon list processing” commit #3000

Open
AlexDaniel opened this issue Jun 19, 2019 · 8 comments
Labels
regression Issue did not exist previously tests needed Issue is generally resolved but tests were not written yet

Comments

@AlexDaniel
Copy link
Contributor

Error message during tests:

===SORRY!===
internal problem: parser did not give circumfix an EXPR
t/00-unpack.t .. 
@AlexDaniel AlexDaniel added regression Issue did not exist previously BLOCKER Preventing the next release of rakudo, or just needing attention before the release labels Jun 19, 2019
@AlexDaniel
Copy link
Contributor Author

Ping @skids

@AlexDaniel AlexDaniel mentioned this issue Jun 19, 2019
17 tasks
@0racle
Copy link
Contributor

0racle commented Jun 21, 2019

The problem seems to be occurring in the return statement of method !unpack inside lib/MessagePack/Unpacker.pm6.

There is a given/when wrapped in parens and used as a return value. Inside the given when is a check for hash key :exists. Something in there is conufising the parser.

Abbreviated version of what it looks like

method !unpack() {
    my $type = self!unpack-uint8;

    return (given $type {
        ...
        when %unpack-for-type{$type} :exists {
            %unpack-for-type{$type}(self);
        }   
        default {
            fail sprintf("Unknown type 0x%02x", $type);
        }   
    }); 
} 

As given returns a value, and it's the last thing in this method, the return is a little unidiomatic. In fact $type is never referred to implicitly inside the given block, so the given is redundant.

In any case, I change return (given $type { ... }); to just given $type { ... }, (or remove the given all togther and just end the method with a string of when clauses and a default clause) the issue goes away and all MessagePack tests pass.

Also, if I leave the return statement as is, but change the condition from when %unpack-for-type{$type}:exists to when (%unpack-for-type{$type}:exists), the issue also goes away and all MessagePack tests pass.

At the very least, a pull-request can be made to fix MessagePack, but hopefully this info might help identify what's confusing the parser.

@0racle
Copy link
Contributor

0racle commented Jun 21, 2019

Golfed...

(when %()<>:exists { ... })

These are ok

when %()<>:exists { ... }
(when (%()<>:exists) { ... })

Also, this issue is not specific to when... if, for or given in place of when still trigger.

@AlexDaniel
Copy link
Contributor Author

I tried to debug this but I'm a bit confused. So $<semilist><statement>[$semi] does exist, but <EXPR> is not quite there. If I .dump $<semilist><statement>[$semi], I get this:

- statement_control: when %()<>:exists { ... }
  - sym: when
  - xblock: %()<>:exists { ... }
    - pblock: { ... }
      - blockoid: { ... }
        - statementlist:  ... 
          - statement: 1 matches
            - EXPR: ... 
              - sym: ...
              - args:  
                - arglist: 
    - EXPR: :exists 
      - 0: <>
        - 0: %()
          - variable: %()
            - contextualizer: %()
              - coercee: 
                - statement:  isa NQPArray
              - sigil: %
              - sequence: 
                - statement:  isa NQPArray
        - OPER: <>
          - nibble: 
          - O: 
        - postfix_prefix_meta_operator:  isa NQPArray
        - postcircumfix: <>
          - nibble: 
          - O: 
      - OPER: 
        - O: 
      - colonpair: :exists
        - identifier: exists
      - fake_infix: 
        - O: 

So the expected EXPR is a bit deeper, but why is it looking for it at all? Could it be that $*FAKE_INFIX_FOUND is not reset correctly?

@AlexDaniel
Copy link
Contributor Author

Ping @skids

@lizmat
Copy link
Contributor

lizmat commented Jun 29, 2019

Fixed for now with c227693 . It spectests clean and makes MessagePack install again with zef. There is probably a better fix possible, but this should unblock the 2019.06 release for now.

@AlexDaniel AlexDaniel added BLOCKER Preventing the next release of rakudo, or just needing attention before the release and removed BLOCKER Preventing the next release of rakudo, or just needing attention before the release labels Jun 29, 2019
@AlexDaniel
Copy link
Contributor Author

Seems like it's not that simple… see https://colabti.org/irclogger/irclogger_log/perl6-dev?date=2019-06-29#l168

The code to trigger the issue gets weirder, but it's still somewhat realistic.

lizmat added a commit that referenced this issue Jun 29, 2019
- pre-check EXPRessions, filter out the undefined ones
- refactor circumfix() and circumfix[] code into single sub
- remove dead code
@lizmat
Copy link
Contributor

lizmat commented Jun 29, 2019

More complete solution pushed with a26e95b

@lizmat lizmat added tests needed Issue is generally resolved but tests were not written yet and removed BLOCKER Preventing the next release of rakudo, or just needing attention before the release labels Jun 29, 2019
vrurg added a commit to vrurg/rakudo that referenced this issue Jul 10, 2019
A more complete solution to rakudo#3000

- pre-check EXPRessions, filter out the undefined ones
- refactor circumfix() and circumfix[] code into single sub
- remove dead code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Issue did not exist previously tests needed Issue is generally resolved but tests were not written yet
Projects
None yet
Development

No branches or pull requests

3 participants