JS: Change pruning to not rely on Import#20297
Merged
asgerf merged 1 commit intogithub:mainfrom Aug 28, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR changes the pruning mechanism for MaD library models to avoid dependency on the Import class, simplifying the dependency graph for constructing DataFlow::Node. The change replaces precise import path detection with an over-approximation based on string literals to break circular dependencies between imports and data flow analysis.
- Removes dependency on
Importclass inisPackageUsedpredicate - Uses string literals as an over-approximation for imported packages
- Adds support for string literal type expressions used in dynamic imports
| package = any(JS::Import imp).getImportedPathString() | ||
| // To simplify which dependencies are needed to construct DataFlow::Node, we don't want to rely on `Import` here. | ||
| // Just check all string literals. | ||
| package = any(JS::Expr imp).getStringValue() |
There was a problem hiding this comment.
Using any(JS::Expr imp).getStringValue() will iterate over all expressions in the codebase to find string values, which could be inefficient. Consider using a more specific expression type like JS::StringLiteral to reduce the search space.
Suggested change
| package = any(JS::Expr imp).getStringValue() | |
| package = any(JS::StringLiteral imp).getStringValue() |
aschackmull
approved these changes
Aug 28, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background: To ensure MaD library models have a near-zero cost for codebases that don't use the modelled library, we prune models based on what packages are imported in the current codebase. This means we don't parse the access paths or synthesise data flow nodes for irrelevant models.
This means that in order to generate
DataFlow::Node, we first have to compute imported paths. There is thus a dependency on theImportclass. However, theImportclass also depends on local data flow. We therefore haveTEarlyStageNode, which is used byImportbut does not contain flow summary-generated nodes.For overlay mode, the
TEarlyStageNodehas no effect on locality as the entire newtype needs to be made local. The dependency above puts us in an "all or nothing" situation where a lot needs to be made local in order forDataFlow::Nodeto become local.In order to simplify the problem, I'm cutting the dependency in this PR, so pruning of flow summaries only depends on an over-approximation, roughly based on what string literals appear in the program.
Note that there are more dependencies to be cut, but I'm trying to split this into small independent PRs.
Evaluation looks neutral.