Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.Sign up
Improve compatibility with MRI's Ripper #4898
After having discovered #4882 and then discovering that (block-)local variable references didn't work, it seemed like the whole process would be quicker if I looked for the solution myself, rather than wait for someone else to solve one problem before I could discover the next one.
And once I had figured out roughly how the code worked, I spent a little time to try to get all the MRI specs to pass. Apart from the tests that rely on the MRI directory layout, the only remaining issue is that JRuby's Ripper doesn't validate regular expressions during parsing. I looked into it briefly, but given that the Ripper parser effectively doesn't have access to a parse tree (as the
In addition to the MRI tests, this adds
Given that this is essentially a collection of a whole bunch of small fixes, I refer to the individual commit messages for additional information, instead of trying to summarize the changes here.
@grddev This is fantastic.
If you have interest in 9.1.x supporting these changes then you might maybe be up for the challenge of opening up another PR for jruby-9.1 branch so our current 9.1.x can get these changes. Note though that a few of these commits will not cherry-pick over (notably RipperParser.y) since ruby 2.4 added new productions 2.3 does not have like arg_rhs (ah also master changed Tokens to use keyword_XXX).
If you don't want to do that work I can try cp'ing as much as is reasonable and then fix the rest.
Another test case I frequently use is Yard although we pass their specs so as you can see ripper coverage is not fantastic.
For regexp checking did you see ParserSupport.regexpFragmentCheck? I have not spent any time looking at this but that method will check syntax but only require a ByteList. For named captures as lvars perhaps that is more involved than that?
Dec 16, 2017
1 check failed
referenced this pull request
Dec 16, 2017
I did see that, but the main part that the fragment validation does is ensure that the encoding is valid in each fragment, which isn't really possible to do before seeing the flags for the regexp, which in turn would require the Parser/Lexer to keep track of the regular expression fragments until getting to the options. Once the fragments are recorded, it would also be relatively simple to perform full validation/compilation of the regexp if the expression is static.
I didn't even think about variable captures, and haven't looked into how MRI Ripper deals with that, so that might require some investigation as well.
Its definitely solvable, but I believe it would be complicated enough to warrant its own PR (and I'm also not sure I have the time to look into it right now).