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

Adds .kind prop to AstNode. #799

Merged
merged 6 commits into from Jun 6, 2023
Merged

Adds .kind prop to AstNode. #799

merged 6 commits into from Jun 6, 2023

Conversation

TwitchBronBron
Copy link
Member

  • Adds a .kind to AstNode, and an AstNodeKind enum.
  • converts the reflection methods to check for .kind instead of the thing?.constructor?.name === 'Whatever' logic.

Raw benchmarks show this as a fairly significant boost:
image

What that means in practice is validation speeds improve by 10-12 percent.

image

Downsides:
This is a breaking change, as all plugins that produce AST would need to be upgraded to the latest version of brighterscript in order to work with the version of brighterscript shipped with the vscode extension. Or, for example, latest version of brighterscript being used with a plugin that depends on an older version of brighterscript.

Converts reflection methods to check `kind` which is faster.
@markwpearce
Copy link
Collaborator

Looks good.
If breaking changes are needed, we shouldn't be afraid of them.

@philippe-wm
Copy link
Contributor

Really welcome improvement - can't say I was happy with the constructor check!

@TwitchBronBron TwitchBronBron changed the base branch from master to release-0.66.0 June 6, 2023 17:57
@TwitchBronBron TwitchBronBron merged commit ae8afa5 into release-0.66.0 Jun 6, 2023
5 checks passed
@TwitchBronBron TwitchBronBron deleted the AstNode.kind branch June 6, 2023 19:28
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.

None yet

3 participants