Skip to content

Conversation

@asgerf
Copy link
Contributor

@asgerf asgerf commented Sep 12, 2025

Adds compileForOverlayEval: true and fixes a bad join order arising when this is enabled.

Evaluations:

@github-actions github-actions bot added the JS label Sep 12, 2025
@asgerf asgerf force-pushed the js/compile-for-overlay-eval branch from 4f55fcb to 33001a7 Compare September 12, 2025 07:52
@asgerf asgerf added the no-change-note-required This PR does not need a change note label Sep 12, 2025
@asgerf asgerf marked this pull request as ready for review September 12, 2025 11:27
@asgerf asgerf requested a review from a team as a code owner September 12, 2025 11:27
Copilot AI review requested due to automatic review settings September 12, 2025 11:27
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

Enables the compileForOverlayEval: true configuration in the JavaScript CodeQL library and optimizes token navigation by introducing a helper predicate to improve join ordering performance.

  • Adds compileForOverlayEval: true to the qlpack.yml configuration
  • Introduces adjacentTokens helper predicate to optimize join order in token navigation
  • Refactors getNextToken() method to use the new helper predicate

Reviewed Changes

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

File Description
javascript/ql/lib/qlpack.yml Enables compileForOverlayEval configuration option
javascript/ql/lib/semmle/javascript/Tokens.qll Adds adjacentTokens helper predicate and refactors getNextToken for better performance

Napalys
Napalys previously approved these changes Sep 15, 2025
@asgerf asgerf marked this pull request as draft September 18, 2025 19:26
@asgerf asgerf force-pushed the js/compile-for-overlay-eval branch from 33001a7 to f4b798b Compare October 22, 2025 09:49
@asgerf asgerf marked this pull request as ready for review October 27, 2025 10:12
@asgerf asgerf marked this pull request as draft October 27, 2025 12:31
@asgerf asgerf force-pushed the js/compile-for-overlay-eval branch from 80475a6 to fdb6dd3 Compare October 27, 2025 12:32
@asgerf asgerf marked this pull request as ready for review October 30, 2025 09:11
tausbn
tausbn previously approved these changes Oct 30, 2025
Copy link
Contributor

@tausbn tausbn left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise this looks good to me. 👍


import javascript

private predicate adjacentTokens(Token token1, Token token2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No pragma[nomagic] on this to prevent future reinlining?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added pragma[nomagic]. The DIL seems to be unchanged so we shouldn't have to rerun the evaluations.

@asgerf asgerf force-pushed the js/compile-for-overlay-eval branch from 120dcd3 to c583b48 Compare October 30, 2025 14:31
@asgerf asgerf merged commit a1a9626 into github:main Oct 31, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JS no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants