Skip to content

Commit

Permalink
fix recording deps to imported-as-object unknown modules
Browse files Browse the repository at this point in the history
Summary: We should be able to record dependencies on unknown modules even when they are imported as an object (e.g. `import unknown_mod`), not only when they are imported-from (e.g. `from unknown_mod import U`). In this case, there's no need to record granular deps, we just need to know we have a dependency on the module.

Reviewed By: DinoV

Differential Revision: D54695279

fbshipit-source-id: 16ba0725ce1031a55af6a269643154ce7505c645
  • Loading branch information
Carl Meyer authored and facebook-github-bot committed Mar 13, 2024
1 parent 79a30fb commit 111b7b5
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
15 changes: 13 additions & 2 deletions cinderx/PythonLib/cinderx/compiler/static/declaration_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ def visitImport(self, node: Import) -> None:
top_level_module,
None,
DeferredImport(
self.module,
name.name,
top_level_module,
self.optimize,
self.compiler,
mod_to_return=top_level_module,
Expand All @@ -246,7 +248,9 @@ def visitImport(self, node: Import) -> None:
self.module.declare_import(
asname,
None,
DeferredImport(name.name, self.optimize, self.compiler),
DeferredImport(
self.module, name.name, asname, self.optimize, self.compiler
),
)

def visitImportFrom(self, node: ImportFrom) -> None:
Expand All @@ -258,7 +262,14 @@ def visitImportFrom(self, node: ImportFrom) -> None:
self.module.declare_import(
child_name,
(mod_name, name.name),
DeferredImport(mod_name, self.optimize, self.compiler, name.name),
DeferredImport(
self.module,
mod_name,
name.name,
self.optimize,
self.compiler,
name.name,
),
)

# We don't pick up declarations in nested statements
Expand Down
9 changes: 9 additions & 0 deletions cinderx/PythonLib/cinderx/compiler/static/module_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,17 @@ def __init__(self, reason: str) -> None:
class DeferredImport:
def __init__(
self,
in_module: ModuleTable,
mod_to_import: str,
asname: str,
optimize: int,
compiler: Compiler,
name: str | None = None,
mod_to_return: str | None = None,
) -> None:
self.in_module = in_module
self.mod_to_import = mod_to_import
self.asname = asname
self.name = name
self.mod_to_return = mod_to_return
self.compiler = compiler
Expand Down Expand Up @@ -241,6 +245,11 @@ def resolve(self) -> Value:
raise ModuleTableException(
f"Cannot find {self.mod_to_import}.{self.name} due to cyclic reference"
)
# For unknown modules, we don't need to record each individual object
# used from them; it doesn't matter, as we won't track transitive deps
# across them. We just need to record a dependency to the module in
# general.
self.in_module.record_dependency(self.asname, (self.mod_to_import, "<any>"))
return self.compiler.type_env.DYNAMIC


Expand Down
31 changes: 23 additions & 8 deletions cinderx/test_cinderx/test_compiler/test_static/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,15 +398,30 @@ def g():

def test_dep_on_unknown_module(self) -> None:
"""We record deps on non-static modules; they could become static."""
code = """
from unknown import U
for from_import in [True, False]:
if from_import:
code = """
from unknown import U
class C(U):
pass
"""
compiler = self.compiler(mod=code)
compiler.compile_module("mod")
self.assertDep(compiler.modules["mod"].decl_deps, "C", {("unknown", "U")})
class C(U):
pass
"""
else:
code = """
import unknown as unk
class C(unk.U):
pass
"""
with self.subTest(from_import=from_import):
compiler = self.compiler(mod=code)
compiler.compile_module("mod")
expected = {("unknown", "U")} if from_import else {("mod", "unk")}
self.assertDep(compiler.modules["mod"].decl_deps, "C", expected)
if not from_import:
self.assertDep(
compiler.modules["mod"].decl_deps, "unk", {("unknown", "<any>")}
)


class GetDependenciesTests(StaticTestBase):
Expand Down

0 comments on commit 111b7b5

Please sign in to comment.