Project

General

Profile

Actions

Feature #20564

open

Switch default parser to Prism

Added by kddnewton (Kevin Newton) 5 months ago. Updated about 2 months ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:118243]

Description

This issue is to propose switching the default parser for Ruby to Prism on the master branch. The main goal is to get this default into both master and the next preview so that the community has plenty of time to test before the eventual 3.4 release.

Matz has indicated that the final decision about the default parser for Ruby 3.4 will be made later in the year. Decisions about the default parser for Ruby 3.5+ and the relationship between Prism, lrama, parse.y, etc. will also be made at a later time.

Background

For some background on the Prism project itself, here are a couple of links in chronological order that you can read:

Status

Prism now passes all Ruby tests and specs. Internally at Shopify, it also passes our Core monolith's CI, which is a large and complex Rails application. We are confident that it is ready for production use. External to CRuby, Prism has already been adopted extensively around the ecosystem, including but not limited to:

Fortunately with the community adoption, we have been able to find many edge cases and bugs that have been fixed. Thank you so much to everyone who has tried, used, reported problems with, and improved Prism in the last couple of years. We would not have hit this milestone without this community involvement.

Caveats

While Prism correctly accepts and parses all valid Ruby syntax, there are still a number of cases where it does not reject invalid syntax as it should. We are currently working on these cases, and we are confident that we can eliminate them before the next preview release. You can find them listed here: https://github.com/ruby/prism/issues?q=is%3Aopen+is%3Aissue+label%3Ainvalid-syntax. This is our top priority right now, and I anticipate these being resolved quickly.

The other caveat is unknown unknowns: bugs or syntax issues that we have yet to discover. At Shopify we are currently taking numerous steps to mitigate this risk, including:

  • Prism currently passes all Ruby tests and specs, and will continue to going forward. The tests and specs are extensive, and we are confident this gets us almost all of the way there.
  • Grammar-based fuzzing to ensure we handle all possible syntax variations (both valid and invalid). As an example, that recently led to the discovery that the current compiler was segfaulting on safe navigation in for loops (https://bugs.ruby-lang.org/issues/20468). It has also found several memory leaks that have been fixed.
  • Mutation testing of existing source files (by adding and deleting random bytes) to find additional edge cases.

Our goal is 100% compatibility, and we believe this is achievable through the above techniques and others. The goal of this proposal is to allow more companies to test their codebase against Prism, and to make this easier. We believe that we can sort out the remaining problems much more quickly in that manner.

Implementation

The implementation will be relatively straightforward, as there is already the --parser flag that can be used to switch between the current parser and Prism. We will change the default to be "prism" instead of "parse.y". There is no plan to remove or change any code related to parse.y, lrama, or compile.c at this time, as we want to create the minimal amount of change possible to ensure stability.

Updated by hsbt (Hiroshi SHIBATA) 5 months ago

Prism now passes all Ruby tests and specs.

No, https://github.com/ruby/ruby/blob/master/.github/workflows/prism.yml#L89 is not testing with make test-syntax-suggest, make test-bundled-gems and make test-bundler-parallel. At least, we should pass them to keep CI green. You may use make check that includes make test-syntax-suggest or make exam that is "all" test of Ruby test suite.

Updated by kddnewton (Kevin Newton) 5 months ago

Ah, thank you @hsbt (Hiroshi SHIBATA). We of course will not merge this until we are sure it is passing 100% of the tests. I think we will likely discover other things as well when we switch the default (before we merge), which is of course what we're looking for!

Updated by kddnewton (Kevin Newton) 5 months ago

Happily I was able to turn all of those tests on with minimal changes. I'll clean this PR up next week, but all should be well with the additional tests (https://github.com/ruby/ruby/pull/10939).

Updated by matz (Yukihiro Matsumoto) 5 months ago

Prism needs quality and for that it needs to be experimented with on a wider scale. For that reason, I agree with making Prism the default in previews (when Prism passes all tests). Whether it will eventually become the default in 3.4 depends on quality.

Matz.

Updated by hsbt (Hiroshi SHIBATA) 4 months ago

Is there the criteria for reverting this until Ruby 3.4 release?

Updated by kddnewton (Kevin Newton) about 2 months ago

  • Status changed from Open to Closed

Per this ticket, I have merged https://github.com/ruby/ruby/pull/11497 to switch the default parser on master. The caveats in this ticket still apply, and I will be working on those next.

As a part of this PR I have added a new GitHub actions workflow to test --parser=parse.y. This will ensure we do not accidentally introduce regressions in the parse.y parser/compiler pair. I have additionally introduced some test exclusions for this parser, as there are a couple of tests that are asserting against RUBY_DESCRIPTION that need to get updated. We're working on getting those merged as soon as possible.

In the meantime, if users are testing against Ruby master (like Shopify) then they will now be able to test Prism's parser/compiler pair, so hopefully we can surface bugs much more quickly and far out enough from the 3.4 release.

Updated by hsbt (Hiroshi SHIBATA) about 2 months ago

  • Status changed from Closed to Open

Is there the criteria for reverting this until Ruby 3.4 release?

I'm still waiting this answer.

Actions

Also available in: Atom PDF

Like3
Like0Like0Like1Like2Like0Like0Like0