-
Notifications
You must be signed in to change notification settings - Fork 1.9k
JS: Make SourceNode::Range non-recursive and make strings SourceNodes #4772
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
Conversation
max-schaefer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor suggestion.
I'd be happy to accept the (apparently minor) performance penalty.
| private class NodeModuleSourcesNodes extends DataFlow::SourceNode::Range { | ||
| NodeModuleSourcesNodes() { | ||
| exists(NodeModule m | | ||
| this = DataFlow::ssaDefinitionNode(SSA::implicitInit([m.getModuleVariable(), m.getExportsVariable()])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth binding the variable to a field and exposing it through a getter? Then we could use that getter in the charpred of {Module,Exports}VarNode to avoid some duplication.
max-schaefer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
384297d to
44b9b62
Compare
|
Superseded by #5499 |
Evaluation (internal link) shows a minor slow down due to optimizer wobbles, but it has been observed to help with the BDD node limit in some cases.