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

Upgrade Chevrotain to 6.x #248

Closed
bd82 opened this issue Aug 27, 2019 · 10 comments
Closed

Upgrade Chevrotain to 6.x #248

bd82 opened this issue Aug 27, 2019 · 10 comments

Comments

@bd82
Copy link
Contributor

bd82 commented Aug 27, 2019

There have been quite a few changes including breaking ones.
https://sap.github.io/chevrotain/docs/changes/CHANGELOG.html#_6-1-0-8-25-2019

@clementdessoude
Copy link
Contributor

I'll try to do the update

@bd82
Copy link
Contributor Author

bd82 commented Aug 27, 2019

You will probably need to wrap the custom backtracking logic with $.ACTION

@bd82
Copy link
Contributor Author

bd82 commented Sep 14, 2019

Version 6.3 includes features and a guide related to improving initialization performance.

@Shaolans
Copy link
Member

With @clement26695, we are looking to update prettier-java to the latest version of Chevrotain.
However we do not completely understand how to use ACTION in our backtracking logic since they are using rules.
Would it be possible to show us how to refactor this logic with the example below ?

  $.RULE("isCastExpression", () => {
    if (this.BACKTRACK_LOOKAHEAD($.isPrimitiveCastExpression)) {
      return true;
    }
    return this.BACKTRACK_LOOKAHEAD($.isReferenceTypeCastExpression);
  });

  $.RULE("isPrimitiveCastExpression", () => {
    $.CONSUME(t.LBrace);
    $.SUBRULE($.primitiveType);
    // No dims so this is not a reference Type
    $.CONSUME(t.RBrace);
    return true;
  });

@bd82
Copy link
Contributor Author

bd82 commented Sep 20, 2019

Try wrapping the whole isCastExpression in "ACTION".
For isPrimitiveCastExpression you should not need to do anything.

  $.RULE("isCastExpression", () => {
    return $.ACTION(() => {
           if (this.BACKTRACK_LOOKAHEAD($.isPrimitiveCastExpression)) {
        return true;
      }
      return this.BACKTRACK_LOOKAHEAD($.isReferenceTypeCastExpression);
      })
  });

@bd82
Copy link
Contributor Author

bd82 commented Sep 20, 2019

Basically you want to avoid running backtracking logic during the grammar recording phase

@clementdessoude
Copy link
Contributor

I have to admit that I am a bit lost here... I just tried to upgrade it and wrapped every Backtracking logic in an ACTION (see here), but I got an error when I try to parse any file, for instance this one:

public class App {

  private static final Logger LOGGER = LoggerFactory.getLogger(App.class);

  public App() {
    LOGGER.info("Constructing parts and car");
  }
}

I got this stacktrace:

TypeError: Cannot read property 'call' of undefined
    at JavaParser.RecognizerEngine.optionInternalLogic (/home/cdessoude/Documents/workplace/prettier-java/node_modules/chevrotain/lib/src/parse/parser/traits/recognizer_engine.js:227:27)
    at JavaParser.RecognizerEngine.optionInternal (/home/cdessoude/Documents/workplace/prettier-java/node_modules/chevrotain/lib/src/parse/parser/traits/recognizer_engine.js:196:25)
    at JavaParser.RecognizerApi.OPTION (/home/cdessoude/Documents/workplace/prettier-java/node_modules/chevrotain/lib/src/parse/parser/traits/recognizer_api.js:102:21)
    at JavaParser.$.ACTION (/home/cdessoude/Documents/workplace/prettier-java/packages/java-parser/src/productions/packages-and-modules.js:215:9)
    at JavaParser.RecognizerApi.ACTION (/home/cdessoude/Documents/workplace/prettier-java/node_modules/chevrotain/lib/src/parse/parser/traits/recognizer_api.js:21:21)
    at JavaParser.$.RULE (/home/cdessoude/Documents/workplace/prettier-java/packages/java-parser/src/productions/packages-and-modules.js:214:14)
    at JavaParser.invokeRuleWithTry (/home/cdessoude/Documents/workplace/prettier-java/node_modules/chevrotain/lib/src/parse/parser/traits/recognizer_engine.js:119:33)
    at JavaParser.wrappedGrammarRule [as isModuleCompilationUnit] (/home/cdessoude/Documents/workplace/prettier-java/node_modules/chevrotain/lib/src/parse/parser/traits/recognizer_engine.js:133:38)
    at JavaParser.BACKTRACK_LOOKAHEAD (/home/cdessoude/Documents/workplace/prettier-java/packages/java-parser/src/parser.js:183:25)
    at JavaParser.$.RULE (/home/cdessoude/Documents/workplace/prettier-java/packages/java-parser/src/productions/packages-and-modules.js:8:24)

@bd82
Copy link
Contributor Author

bd82 commented Sep 23, 2019

  • Never wrap DSL methods (CONSUME/OPTION/SUBRULE/...) in ACTION.
  • Only wrap the invocation to the backtracking utilities in ACTION.

Bad example: https://github.com/clement26695/prettier-java/pull/4/files#diff-3be86ec11b8168a43f3526a40d5af78eR214

Basically You want the "recording phase" to run on your rules to build the data structure representing the grammar. so it cannot be wrapped in ACTION as this skips its invocation during the recording phase. However you do not want to RECORD the backtracking logic as that is not part of the grammar, so you wrap calls to isXYZ in ACTION.

btw: GATES are not executed during backtracking so no need to wrap code already inside GATE property with ACTION.

This is a pretty complex grammar in terms of using advanced optimized backtracking...
If you are still stuck I will try to convert myself, perhaps on the weekend...

@clementdessoude
Copy link
Contributor

Thank you for the clarifications ! Will try to upgrade this week, I will keep you posted

@clementdessoude
Copy link
Contributor

Closed with #268

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants