Follow re-export chains when checking call safety#8
Open
msullivan wants to merge 1 commit intofacebook:mainfrom
Open
Follow re-export chains when checking call safety#8msullivan wants to merge 1 commit intofacebook:mainfrom
msullivan wants to merge 1 commit intofacebook:mainfrom
Conversation
Re-exported symbols (e.g. bar.Foo re-exporting foo.Foo) were not being resolved to their original definitions during safety analysis. This caused check_call to miss unsafe constructors/functions accessed through re-exports, incorrectly marking them as safe. Change re_exports from a HashSet to a HashMap that preserves the mapping to original names, add resolve_re_export() that follows chains, and apply it in check_call and the ImportedTypeAttr property lookup.
Contributor
|
it might be possible to do this in the alias/bindings table, since those have access to the global |
meta-codesync Bot
pushed a commit
that referenced
this pull request
Apr 28, 2026
Summary: This is a correctness fix for transitive reexports. #8 highlighted some issues with the way we handle these, but only put in the top-level fix to handle function calls; I'm going through the stack to see where else we should be handling transitive reexports in the lower-level data structures. Reviewed By: brittanyrey Differential Revision: D102414882 fbshipit-source-id: 0a811e2dfa00efbff8f4df58a50983518ac80c7e
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
2 participants
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.
Re-exported symbols (e.g. bar.Foo re-exporting foo.Foo) were not being
resolved to their original definitions during safety analysis. This
caused check_call to miss unsafe constructors/functions accessed through
re-exports, incorrectly marking them as safe.
Make the re_exports more visible to the main analysis, and resolve
them when needed.
I'm not totally happy with this approach, though, because it's would
be easy to miss new places that a reexport needs to be chased down.
I'll do a bit more thinking, but I'd be interested in architectural
suggestions from the team.