X Tutup
The Wayback Machine - https://web.archive.org/web/20260105003833/https://github.com/github/codeql/pull/15966
Skip to content

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
@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.

2 participants

X Tutup