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

Execute ComponentPass passes according to the dependencies between components #2010

Merged
merged 6 commits into from
Jan 31, 2025

Conversation

oxisto
Copy link
Member

@oxisto oxisto commented Jan 30, 2025

This PR makes the same import-dependencies object that is currently available on a Component (between TUs) available on the TranslationResult (between components). I decided to generalize the ImportDependencies class for this. This might sounds overkill but my feeling is that this might pay off at some point.

To access that information, I added two new "strategies" (TRANSLATION_UNITS_LEAST_IMPORTS and COMPONENTS_LEAST_IMPORTS). I do not really like the UPPERCASE naming on these strategies, but we probably want to change that for all of them at some point. So they are named like this for consistency.

Furthermore, ComponentPass passes are now executed according to the COMPONENTS_LEAST_IMPORTS strategy and a TranslationUnitPass is executed according to first the COMPONENTS_LEAST_IMPORTS and then the TRANSLATION_UNITS_LEAST_IMPORTS inside the component. This should make it harmonised for all passes.

The most important pass to use that is the SymbolResolver, which now resolves symbols of components in the desired order as preparation needed for #2006.

Because the ImportResolver populates this map, it cannot use this strategy and I had to promote this to a TranslationResultPass, which I think is ok. It still gathers the information on a component level, just the entry point is the translation result.

@oxisto oxisto linked an issue Jan 30, 2025 that may be closed by this pull request
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

Attention: Patch coverage is 87.32394% with 9 lines in your changes missing coverage. Please review.

Project coverage is 78.09%. Comparing base (7495654) to head (aff15bc).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...n/de/fraunhofer/aisec/cpg/passes/ImportResolver.kt 85.41% 1 Missing and 6 partials ⚠️
...aunhofer/aisec/cpg/processing/strategy/Strategy.kt 84.61% 0 Missing and 2 partials ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
...otlin/de/fraunhofer/aisec/cpg/TranslationResult.kt 75.75% <100.00%> (+0.75%) ⬆️
.../kotlin/de/fraunhofer/aisec/cpg/graph/Component.kt 90.00% <ø> (-0.91%) ⬇️
...kotlin/de/fraunhofer/aisec/cpg/graph/Extensions.kt 63.85% <100.00%> (+0.08%) ⬆️
...main/kotlin/de/fraunhofer/aisec/cpg/passes/Pass.kt 73.60% <100.00%> (+4.74%) ⬆️
...n/de/fraunhofer/aisec/cpg/passes/SymbolResolver.kt 88.02% <100.00%> (-0.05%) ⬇️
...aunhofer/aisec/cpg/processing/strategy/Strategy.kt 70.00% <84.61%> (+27.14%) ⬆️
...n/de/fraunhofer/aisec/cpg/passes/ImportResolver.kt 89.52% <85.41%> (-4.73%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KuechA KuechA changed the title Execeute ComponentPass passes according to the dependencies between components Execute ComponentPass passes according to the dependencies between components Jan 30, 2025
@oxisto oxisto force-pushed the component-dependencies branch from 43fcc23 to f8c0d16 Compare January 30, 2025 15:31
@oxisto oxisto marked this pull request as ready for review January 30, 2025 15:53
@oxisto oxisto force-pushed the component-dependencies branch from a4ad5d1 to e22c6cf Compare January 31, 2025 14:49
@oxisto oxisto enabled auto-merge (squash) January 31, 2025 15:11
@oxisto oxisto force-pushed the component-dependencies branch from e22c6cf to aff15bc Compare January 31, 2025 15:11
@oxisto oxisto merged commit 4b8042a into main Jan 31, 2025
2 checks passed
@oxisto oxisto deleted the component-dependencies branch January 31, 2025 15:23
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.

Propagate import dependencies to components
2 participants