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

Tree-sitter: Split up ast_node_info table into two tables #15966

Merged
merged 4 commits into from
Mar 20, 2024

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Mar 19, 2024

The DIL generated for AstNode.getParent is unnecessarily complex. Instead of using the ast_node_info extensional directly, we get a join with

TreeSitter::Ruby::AstNode#941e5e6b#b(
  /* TreeSitter::Ruby::AstNode.extends */ interned unique entity this
)
{
  (
    exists(
      /* @ruby_underscore_method_name */ interned dontcare entity _,
      /* @ruby_underscore_method_name */ interned dontcare entity _1
     |
      ruby_alias_def(this, _, _1)
    ) and
    project#ruby_ast_node_info(this)
  )
  or
  (ruby_alternative_pattern_def(this) and project#ruby_ast_node_info(this))
  // or
  // ... one case for each member @ruby_ast_node

This happens because ast_node_info needs to contain a row for each AST node, as it also records AST node locations, and hence the column for the parent is not simply an AstNode, but either an AstNode or a File (to account for root AST nodes). By splitting up ast_node_info into two separate extensionals, ast_node_location and ast_node_parent, we can use the ast_node_parent extensional directly in AstNode.getParent.

The decrease in DIL size is confirmed by DCA.

@hvitved hvitved force-pushed the treesitter-split-up-node-info-table branch from 13b3d72 to b4705a3 Compare March 19, 2024 09:53
@github-actions github-actions bot added the Ruby label Mar 19, 2024
@hvitved hvitved force-pushed the treesitter-split-up-node-info-table branch 2 times, most recently from 61f8c32 to f71b8f9 Compare March 19, 2024 09:58
@hvitved hvitved force-pushed the treesitter-split-up-node-info-table branch 4 times, most recently from 0f636bb to ab01ced Compare March 19, 2024 11:55
@hvitved hvitved force-pushed the treesitter-split-up-node-info-table branch from ab01ced to 31e0463 Compare March 19, 2024 12:05
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Mar 19, 2024
@hvitved hvitved marked this pull request as ready for review March 19, 2024 15:02
@hvitved hvitved requested review from a team as code owners March 19, 2024 15:02
Copy link
Contributor

@nickrolfe nickrolfe left a comment

Choose a reason for hiding this comment

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

Interesting. I wonder why the original ruby_ast_node_parent table (before #7875 renamed it and added the location column) could represent file-parents if we never relied on it in QL.

Anyway, the changes LGTM.

@hvitved hvitved removed request for a team March 20, 2024 19:38
@hvitved hvitved merged commit 8f56ede into github:main Mar 20, 2024
42 checks passed
@hvitved hvitved deleted the treesitter-split-up-node-info-table branch March 20, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note QL-for-QL Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants