[compiler] Infer optional dependencies (behind feature flag)#30819
Merged
josephsavona merged 10 commits intogh/josephsavona/53/basefrom Aug 28, 2024
Merged
[compiler] Infer optional dependencies (behind feature flag)#30819josephsavona merged 10 commits intogh/josephsavona/53/basefrom
josephsavona merged 10 commits intogh/josephsavona/53/basefrom
Conversation
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.
Stack from ghstack (oldest at bottom):
Adds a new feature flag,
@enableOptionalDependencieswhich when enabled allows PropagateScopeDeps and DeriveMinimalDeps to infer optional dependency paths (a?.b).In PropagateScopeDeps we look for specific safe patterns of nested optional member expressions:
<variable> "." / "?." <property><nested> "." / "?." <property>When we find this pattern we record a dependency on the overall chain, so for example in
a?.b.c?.d.map()we would record a dependency ona?.b.c?.d(because the outer.map()portion doesn't match the above structure). If the structure doesn't match - for example witha?.b?.[foo(bar)]?.z- then we fall back to the existing behavior which treats everything after the initial portion as conditional (for that last example we'd continue to recordaas the dep).The other portion is DeriveMinimalDeps, which now represents optional access/dependency states and updates the merge logic to handle them. The order of precedence is unconditional > optional > conditional.
Note: the flag is off by default, but i tried enabling it for all fixtures and the results were all either unchanged or had correct optional deps inferred. I'll also try running it internally with the flag enabled, but i think we can proceed with review and fix-forward any issues identified from that testing.