Skip to content

Commit

Permalink
[mypyc] Fix is_native_ref_expr for class attrs (#18031)
Browse files Browse the repository at this point in the history
This addresses mypyc/mypyc#932 in which
in some cases before we were incorrectly not determining that the symbol
was native.

Because the problematic implementation of `is_native_ref_expr` is
present in multiple places,
the one in `builder.py` is rewritten to use the one in `mapper.py`.

Additional run tests are added to cover the case. Use of a dictionary
here prevents falling
under a secondary, more optimized codepath, that avoided the issue
entirely.
  • Loading branch information
jhance authored Oct 25, 2024
1 parent 776d01d commit 376e2ad
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 8 deletions.
10 changes: 3 additions & 7 deletions mypyc/irbuild/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -979,17 +979,13 @@ def _analyze_iterable_item_type(self, expr: Expression) -> Type:

def is_native_module(self, module: str) -> bool:
"""Is the given module one compiled by mypyc?"""
return module in self.mapper.group_map
return self.mapper.is_native_module(module)

def is_native_ref_expr(self, expr: RefExpr) -> bool:
if expr.node is None:
return False
if "." in expr.node.fullname:
return self.is_native_module(expr.node.fullname.rpartition(".")[0])
return True
return self.mapper.is_native_ref_expr(expr)

def is_native_module_ref_expr(self, expr: RefExpr) -> bool:
return self.is_native_ref_expr(expr) and expr.kind == GDEF
return self.mapper.is_native_module_ref_expr(expr)

def is_synthetic_type(self, typ: TypeInfo) -> bool:
"""Is a type something other than just a class we've created?"""
Expand Down
4 changes: 3 additions & 1 deletion mypyc/irbuild/mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def __init__(self, group_map: dict[str, str | None]) -> None:
self.group_map = group_map
self.type_to_ir: dict[TypeInfo, ClassIR] = {}
self.func_to_decl: dict[SymbolNode, FuncDecl] = {}
self.symbol_fullnames: set[str] = set()

def type_to_rtype(self, typ: Type | None) -> RType:
if typ is None:
Expand Down Expand Up @@ -217,7 +218,8 @@ def is_native_ref_expr(self, expr: RefExpr) -> bool:
if expr.node is None:
return False
if "." in expr.node.fullname:
return self.is_native_module(expr.node.fullname.rpartition(".")[0])
name = expr.node.fullname.rpartition(".")[0]
return self.is_native_module(name) or name in self.symbol_fullnames
return True

def is_native_module_ref_expr(self, expr: RefExpr) -> bool:
Expand Down
2 changes: 2 additions & 0 deletions mypyc/irbuild/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ def build_type_map(
if not options.global_opts:
class_ir.children = None
mapper.type_to_ir[cdef.info] = class_ir
mapper.symbol_fullnames.add(class_ir.fullname)

# Populate structural information in class IR for extension classes.
for module, cdef in classes:
Expand Down Expand Up @@ -149,6 +150,7 @@ def load_type_map(mapper: Mapper, modules: list[MypyFile], deser_ctx: DeserMaps)
if isinstance(node.node, TypeInfo) and is_from_module(node.node, module):
ir = deser_ctx.classes[node.node.fullname]
mapper.type_to_ir[node.node] = ir
mapper.symbol_fullnames.add(node.node.fullname)
mapper.func_to_decl[node.node] = ir.ctor

for module in modules:
Expand Down
12 changes: 12 additions & 0 deletions mypyc/test-data/run-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -2594,3 +2594,15 @@ def test_class_final_attribute_inherited() -> None:
assert C().b == 2
assert B().c == 3
assert C().c == 3

[case testClassWithFinalAttributeAccess]
from typing import Final

class C:
a: Final = {'x': 'y'}
b: Final = C.a

def test_final_attribute() -> None:
assert C.a['x'] == 'y'
assert C.b['x'] == 'y'
assert C.a is C.b

0 comments on commit 376e2ad

Please sign in to comment.