Skip to content

Conversation

@camchenry
Copy link
Member

@camchenry camchenry commented Aug 16, 2025

I decided to revive this stack: #12227 and merge all of the changes into one PR for slightly easier review and changes.

This PR adds a new AstTypesBitset struct which is a set of bits that has a fixed size known at compile time and can be used to store AST types as a set. The aim of this structure is to making checks like "does this file have a for statement?" possible to answer without doing any additional iterations over the AST.

@github-actions github-actions bot added A-semantic Area - Semantic A-ast Area - AST A-ast-tools Area - AST tools C-enhancement Category - New feature or request labels Aug 16, 2025
Copy link
Member Author

camchenry commented Aug 16, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 16, 2025

CodSpeed Instrumentation Performance Report

Merging #13137 will not alter performance

Comparing 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds (f00adbe) with main (5648b31)

Summary

✅ 4 untouched benchmarks
⁉️ 33 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
⁉️ codegen[RadixUIAdoptionSection.jsx] 120.1 µs N/A N/A
⁉️ codegen[binder.ts] 4 ms N/A N/A
⁉️ codegen[cal.com.tsx] 34.4 ms N/A N/A
⁉️ codegen[react.development.js] 1.9 ms N/A N/A
⁉️ formatter[RadixUIAdoptionSection.jsx] 570.9 µs N/A N/A
⁉️ formatter[binder.ts] 22.6 ms N/A N/A
⁉️ formatter[cal.com.tsx] 206.9 ms N/A N/A
⁉️ formatter[react.development.js] 11.5 ms N/A N/A
⁉️ lexer[RadixUIAdoptionSection.jsx] 20.1 µs N/A N/A
⁉️ lexer[binder.ts] 870.5 µs N/A N/A
⁉️ lexer[cal.com.tsx] 5.3 ms N/A N/A
⁉️ lexer[react.development.js] 356.9 µs N/A N/A
⁉️ linter[RadixUIAdoptionSection.jsx] 2.6 ms N/A N/A
⁉️ linter[binder.ts] 148 ms N/A N/A
⁉️ linter[cal.com.tsx] 1.2 s N/A N/A
⁉️ linter[react.development.js] 52.6 ms N/A N/A
⁉️ mangler[RadixUIAdoptionSection.jsx] 11.6 µs N/A N/A
⁉️ mangler[binder.ts] 733.3 µs N/A N/A
⁉️ mangler[cal.com.tsx] 2.8 ms N/A N/A
⁉️ mangler[react.development.js] 259.1 µs N/A N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@camchenry camchenry force-pushed the 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds branch from 55ccbf2 to 8e31c70 Compare August 16, 2025 04:22
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

this looks really nice, should definetly help optimizing linter perf

@camchenry camchenry force-pushed the 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds branch 5 times, most recently from 4649cd4 to af12449 Compare August 23, 2025 17:48
@camchenry camchenry force-pushed the 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds branch from af12449 to f6b3a21 Compare August 24, 2025 04:07
@camchenry camchenry marked this pull request as ready for review August 25, 2025 03:12
Copilot AI review requested due to automatic review settings August 25, 2025 03:12
@camchenry camchenry requested a review from Dunqing as a code owner August 25, 2025 03:12
@camchenry camchenry force-pushed the 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds branch from f6b3a21 to 99956af Compare August 25, 2025 04:03

This comment was marked as outdated.

@camc314 camc314 requested a review from Copilot August 25, 2025 12:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new AstTypesBitset data structure to efficiently track which AST node types are present in a semantic analysis tree. The bitset enables fast "contains" queries without iterating through the entire AST.

  • Introduces AstTypesBitset struct with bitwise operations for AST type tracking
  • Adds AST_TYPE_MAX constant for compile-time bitset sizing
  • Integrates bitset into AstNodes with automatic population during node creation

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

File Description
tasks/ast_tools/src/generators/ast_kind.rs Generates AST_TYPE_MAX constant for bitset sizing
crates/oxc_semantic/src/node.rs Integrates bitset into AstNodes and adds query methods
crates/oxc_semantic/src/lib.rs Adds new module declaration
crates/oxc_semantic/src/ast_types_bitset.rs Implements the core bitset data structure and operations
Comments suppressed due to low confidence (1)

crates/oxc_semantic/src/ast_types_bitset.rs:1

  • The contains method can short-circuit early when a mismatch is found. Consider returning false immediately when mismatch != 0 instead of accumulating all mismatches.
use oxc_ast::{AstType, ast_kind::AST_TYPE_MAX};

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@camchenry camchenry force-pushed the 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds branch 2 times, most recently from 589a962 to f03068b Compare August 26, 2025 01:45
Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

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

Feel free to correct the comment in a later PR, rather than having to restack this whole stack.

@camchenry camchenry force-pushed the 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds branch 4 times, most recently from 6948775 to 9c9a1c9 Compare September 3, 2025 21:31
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Sep 7, 2025
Copy link
Member

overlookmotel commented Sep 7, 2025

Merge activity

…13137)

I decided to revive this stack: #12227 and merge all of the changes into one PR for slightly easier review and changes.

This PR adds a new `AstTypesBitset` struct which is a set of bits that has a fixed size known at compile time and can be used to store AST types as a set. The aim of this structure is to making checks like "does this file have a for statement?" possible to answer without doing any additional iterations over the AST.
@graphite-app graphite-app bot force-pushed the 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds branch from 9c9a1c9 to f00adbe Compare September 7, 2025 07:20
@graphite-app graphite-app bot merged commit f00adbe into main Sep 7, 2025
17 of 25 checks passed
@graphite-app graphite-app bot deleted the 08-15-feat_semantic_add_ability_to_lookup_if_ast_contains_any_node_kinds branch September 7, 2025 07:26
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Sep 7, 2025
graphite-app bot pushed a commit that referenced this pull request Sep 7, 2025
Copilot AI pushed a commit that referenced this pull request Sep 8, 2025
…13137)

I decided to revive this stack: #12227 and merge all of the changes into one PR for slightly easier review and changes.

This PR adds a new `AstTypesBitset` struct which is a set of bits that has a fixed size known at compile time and can be used to store AST types as a set. The aim of this structure is to making checks like "does this file have a for statement?" possible to answer without doing any additional iterations over the AST.
Copilot AI pushed a commit that referenced this pull request Sep 8, 2025
graphite-app bot pushed a commit that referenced this pull request Sep 8, 2025
…le (#13595)

Follow-on after #13137. That PR added methods to `AstNodes`, and doc comments include examples.

Make those examples into proper runnable doctests, and fix them (`AstType::ExportDeclaration` doesn't exist).
graphite-app bot pushed a commit that referenced this pull request Oct 12, 2025
…evant nodes (#14457)

Finally making use of the optimizations enabled by #13137. We can use the AST type bitset for each file to skip rules that only care about specific node types if the file has none of those node types.
camc314 pushed a commit that referenced this pull request Oct 13, 2025
…evant nodes (#14457)

Finally making use of the optimizations enabled by #13137. We can use the AST type bitset for each file to skip rules that only care about specific node types if the file has none of those node types.
camc314 pushed a commit that referenced this pull request Oct 13, 2025
…evant nodes (#14457)

Finally making use of the optimizations enabled by #13137. We can use the AST type bitset for each file to skip rules that only care about specific node types if the file has none of those node types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-ast-tools Area - AST tools A-semantic Area - Semantic C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants