ast: New parser implementation #2219
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
srenatus
reviewed
Mar 24, 2020
CHANGELOG.md
Outdated
| The new parser contains a small number of backwards incompatible changes that | ||
| correct questionable behaviour from the old parser. These changes affect | ||
| a very small number of actual policies and we feel confident in the decision to | ||
| break backwards compatiblity here. |
There was a problem hiding this comment.
typo
Suggested change
| break backwards compatiblity here. | |
| break backwards compatibility here. |
| return isDecimal(ch) || ch >= utf8.RuneSelf && unicode.IsDigit(ch) | ||
| } | ||
|
|
||
| func isDecimal(ch rune) bool { return '0' <= ch && ch <= '9' } |
There was a problem hiding this comment.
This passes gofmt? I wouldn't have thought so 💡
srenatus
reviewed
Mar 24, 2020
| @@ -1,311 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
💭 Do we have a valid grammar somewhere? Could be useful to library authors processing Rego code.
There was a problem hiding this comment.
There is still one in the language reference documentation https://github.com/open-policy-agent/opa/blob/master/docs/content/policy-reference.md#grammar
| name: "Number", | ||
| }, | ||
| &ruleRefExpr{ | ||
| pos: position{line: 198, col: 20, offs |
There was a problem hiding this comment.
This use of defer is incredibly smart.
This commit replaces the existing PEG generated parser with a parser
implemented by hand. The new parser is more efficient (avoiding old
problems with pathological input cases like {{{{{{{{{}}}}}}}} and
deeply-nested composites in general) and offers better opportunities
for improved error reporting (which has been improved already but
there is still room to grow.)
During the test process of implementing the new parser, we identified
a few issues that were present in the old parser. Those issues are
fixed by this commit.
Fixes open-policy-agent#1251
Fixes open-policy-agent#501
Fixes open-policy-agent#2198
Fixes open-policy-agent#2199
Fixes open-policy-agent#2200
Fixes open-policy-agent#2201
Fixes open-policy-agent#2202
Fixes open-policy-agent#2203
Co-authored-by: Torin Sandall <torinsandall@gmail.com>
Co-authored-by: Patrick East <east.patrick@gmail.com>
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Signed-off-by: Patrick East <east.patrick@gmail.com>
|
I've updated the changelog to include the benchstat output against master. |
patrick-east
approved these changes
Mar 25, 2020
jaspervdj-luminal
approved these changes
Mar 26, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit replaces the existing PEG generated parser with a parser
implemented by hand. The new parser is more efficient (avoiding old
problems with pathological input cases like {{{{{{{{{}}}}}}}} and
deeply-nested composites in general) and offers better opportunities
for improved error reporting (which has been improved already but
there is still room to grow.)
During the test process of implementing the new parser, we identified
a few issues that were present in the old parser. Those issues are
fixed by this commit.
Fixes #1251
Fixes #501
Fixes #2198
Fixes #2199
Fixes #2200
Fixes #2201
Fixes #2202
Fixes #2203
Co-authored-by: Torin Sandall torinsandall@gmail.com
Co-authored-by: Patrick East east.patrick@gmail.com
Signed-off-by: Torin Sandall torinsandall@gmail.com
Signed-off-by: Patrick East east.patrick@gmail.com