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

refactor: programs as delimited sequences of statements #164

Merged
merged 2 commits into from
Jun 25, 2023

Conversation

dmfay
Copy link
Collaborator

@dmfay dmfay commented Jun 24, 2023

eliminates the recently added conflicts, shaves a few seconds off compile time, and state count in the parser is cut by over half -- from 7151 states to 3087!

Other changes:

  • compound_statement has been renamed to block
  • create_function uses object_reference correctly
  • we've lost parsing of 'string' function bodies but that was probably a bad idea to begin with

we've lost parsing of string function bodies but that was probably a bad
idea to begin with.
Copy link
Collaborator

@matthias-Q matthias-Q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I am really surprised hat these few change have such a big impact. I am really impressed! Thanks @dmfay

@@ -489,13 +492,48 @@ module.exports = grammar({
// https://stackoverflow.com/questions/13014947/regex-to-match-a-c-style-multiline-comment
marginalia: _ => seq('/*', /[^*]*\*+(?:[^/*][^*]*\*+)*/, '/' ),

compound_statement: $ => seq(
Copy link
Collaborator

@matthias-Q matthias-Q Jun 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In nvim-treesitter there is a indentens.scm, which refenrences compound_Statement. We should have added a copy of this to our repo as well, so that the tests fail here when we rename nodes.

@matthias-Q
Copy link
Collaborator

matthias-Q commented Jun 25, 2023

we've lost parsing of 'string' function bodies but that was probably a bad idea to begin with

what statements are you referring to? Can you give me an example?

@dmfay
Copy link
Collaborator Author

dmfay commented Jun 25, 2023

we've lost parsing of 'string' function bodies but that was probably a bad idea to begin with

what statements are you referring to? Can you give me an example?

there are a couple in the tests, for example

create or replace function public.fn()
  returns int
  language sql
as 'select 1;';

select 1; is now parsed as a literal instead of a statement in its own right. It could be a program if we could ensure that any single quotes internal to it didn't parse in the outer program, which is probably possible but gets complicated since return is a valid statement in a function body but nowhere else.

Copy link
Owner

@DerekStride DerekStride left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow this is great, the decrease in number of states is huge 🎉🎉🎉

@dmfay dmfay merged commit ddd1763 into main Jun 25, 2023
4 checks passed
@dmfay dmfay deleted the dmf/programs-statements branch June 25, 2023 14:29
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

Successfully merging this pull request may close these issues.

3 participants