From ed6b51a58d6734c57da78b05e67200f3365641e7 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 22 Dec 2022 11:26:00 +0000 Subject: [PATCH 1/2] Refactoring: make the type of fullname str instead of Bogus[str] The type Bogus[X] is treated as Any when the code is compiled with mypyc, while it's equivalent to X when only type checking. They are sometimes used when X is not actually the real type of a value, but changing it to the correct type would be complicated. Bogus[str] types are pretty awkward, since we are lying to the type checker and mypyc only sees Any types. An empty fullname is now represented by "" instead of None, so we no longer need a Bogus[str] type. (Various small optimizations, including this, together netted a 6% performance improvement in self check.) --- mypy/checker.py | 4 +- mypy/checkexpr.py | 10 ++--- mypy/nodes.py | 58 +++++++++++++++------------ mypy/partially_defined.py | 2 +- mypy/semanal.py | 12 +++--- mypy/server/aststrip.py | 2 +- mypy/server/deps.py | 12 ++---- mypy/strconv.py | 2 +- mypy/stubtest.py | 2 +- mypy/test/testsemanal.py | 2 +- mypy/typeanal.py | 2 +- mypyc/irbuild/builder.py | 4 +- test-data/unit/plugins/customentry.py | 2 +- 13 files changed, 59 insertions(+), 55 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 065758cd2be9..61104756b297 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2178,7 +2178,7 @@ def visit_class_def(self, defn: ClassDef) -> None: temp = self.temp_node(sig, context=decorator) fullname = None if isinstance(decorator, RefExpr): - fullname = decorator.fullname + fullname = decorator.fullname or None # TODO: Figure out how to have clearer error messages. # (e.g. "class decorator must be a function that accepts a type." @@ -4598,7 +4598,7 @@ def visit_decorator(self, e: Decorator) -> None: temp = self.temp_node(sig, context=e) fullname = None if isinstance(d, RefExpr): - fullname = d.fullname + fullname = d.fullname or None # if this is a expression like @b.a where b is an object, get the type of b # so we can pass it the method hook in the plugins object_type: Type | None = None diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index e6634e124d30..395645b6bf25 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -216,8 +216,8 @@ def extract_refexpr_names(expr: RefExpr) -> set[str]: Note that currently, the only two subclasses of RefExpr are NameExpr and MemberExpr.""" output: set[str] = set() - while isinstance(expr.node, MypyFile) or expr.fullname is not None: - if isinstance(expr.node, MypyFile) and expr.fullname is not None: + while isinstance(expr.node, MypyFile) or expr.fullname: + if isinstance(expr.node, MypyFile) and expr.fullname: # If it's None, something's wrong (perhaps due to an # import cycle or a suppressed error). For now we just # skip it. @@ -228,7 +228,7 @@ def extract_refexpr_names(expr: RefExpr) -> set[str]: if isinstance(expr.node, TypeInfo): # Reference to a class or a nested class output.update(split_module_names(expr.node.module_name)) - elif expr.fullname is not None and "." in expr.fullname and not is_suppressed_import: + elif "." in expr.fullname and not is_suppressed_import: # Everything else (that is not a silenced import within a class) output.add(expr.fullname.rsplit(".", 1)[0]) break @@ -526,7 +526,7 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) -> # There are two special cases where plugins might act: # * A "static" reference/alias to a class or function; # get_function_hook() will be invoked for these. - fullname = e.callee.fullname + fullname = e.callee.fullname or None if isinstance(e.callee.node, TypeAlias): target = get_proper_type(e.callee.node.target) if isinstance(target, Instance): @@ -5489,7 +5489,7 @@ def type_info_from_type(typ: Type) -> TypeInfo | None: def is_operator_method(fullname: str | None) -> bool: - if fullname is None: + if not fullname: return False short_name = fullname.split(".")[-1] return ( diff --git a/mypy/nodes.py b/mypy/nodes.py index 80ab787f4a9c..85bb9ce4a8de 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -25,7 +25,6 @@ from mypy_extensions import trait import mypy.strconv -from mypy.bogus_type import Bogus from mypy.util import short_type from mypy.visitor import ExpressionVisitor, NodeVisitor, StatementVisitor @@ -247,12 +246,10 @@ class SymbolNode(Node): def name(self) -> str: pass - # fullname can often be None even though the type system - # disagrees. We mark this with Bogus to let mypyc know not to - # worry about it. + # Fully qualified name @property @abstractmethod - def fullname(self) -> Bogus[str]: + def fullname(self) -> str: pass @abstractmethod @@ -294,7 +291,7 @@ class MypyFile(SymbolNode): __match_args__ = ("name", "path", "defs") # Fully qualified module name - _fullname: Bogus[str] + _fullname: str # Path to the file (empty string if not known) path: str # Top-level definitions and statements @@ -361,7 +358,7 @@ def name(self) -> str: return "" if not self._fullname else self._fullname.split(".")[-1] @property - def fullname(self) -> Bogus[str]: + def fullname(self) -> str: return self._fullname def accept(self, visitor: NodeVisitor[T]) -> T: @@ -526,8 +523,7 @@ def __init__(self) -> None: self.is_static = False self.is_final = False # Name with module prefix - # TODO: Type should be Optional[str] - self._fullname = cast(Bogus[str], None) + self._fullname = "" @property @abstractmethod @@ -535,7 +531,7 @@ def name(self) -> str: pass @property - def fullname(self) -> Bogus[str]: + def fullname(self) -> str: return self._fullname @@ -871,7 +867,7 @@ def name(self) -> str: return self.func.name @property - def fullname(self) -> Bogus[str]: + def fullname(self) -> str: return self.func.fullname @property @@ -967,7 +963,7 @@ def __init__(self, name: str, type: mypy.types.Type | None = None) -> None: super().__init__() self._name = name # Name without module prefix # TODO: Should be Optional[str] - self._fullname = cast("Bogus[str]", None) # Name with module prefix + self._fullname = "" # Name with module prefix # TODO: Should be Optional[TypeInfo] self.info = VAR_NO_INFO self.type: mypy.types.Type | None = type # Declared or inferred type, or None @@ -1019,7 +1015,7 @@ def name(self) -> str: return self._name @property - def fullname(self) -> Bogus[str]: + def fullname(self) -> str: return self._fullname def accept(self, visitor: NodeVisitor[T]) -> T: @@ -1057,7 +1053,7 @@ class ClassDef(Statement): __slots__ = ( "name", - "fullname", + "_fullname", "defs", "type_vars", "base_type_exprs", @@ -1075,7 +1071,7 @@ class ClassDef(Statement): __match_args__ = ("name", "defs") name: str # Name of the class without module prefix - fullname: Bogus[str] # Fully qualified name of the class + _fullname: str # Fully qualified name of the class defs: Block type_vars: list[mypy.types.TypeVarLikeType] # Base class expressions (not semantically analyzed -- can be arbitrary expressions) @@ -1102,7 +1098,7 @@ def __init__( ) -> None: super().__init__() self.name = name - self.fullname = None # type: ignore[assignment] + self._fullname = "" self.defs = defs self.type_vars = type_vars or [] self.base_type_exprs = base_type_exprs or [] @@ -1117,6 +1113,14 @@ def __init__( self.deco_line: int | None = None self.removed_statements = [] + @property + def fullname(self) -> str: + return self._fullname + + @fullname.setter + def fullname(self, v: str) -> None: + self._fullname = v + def accept(self, visitor: StatementVisitor[T]) -> T: return visitor.visit_class_def(self) @@ -1725,7 +1729,7 @@ class RefExpr(Expression): __slots__ = ( "kind", "node", - "fullname", + "_fullname", "is_new_def", "is_inferred_def", "is_alias_rvalue", @@ -1739,7 +1743,7 @@ def __init__(self) -> None: # Var, FuncDef or TypeInfo that describes this self.node: SymbolNode | None = None # Fully qualified name (or name if not global) - self.fullname: str | None = None + self._fullname = "" # Does this define a new name? self.is_new_def = False # Does this define a new name with inferred type? @@ -1752,6 +1756,14 @@ def __init__(self) -> None: # Cache type guard from callable_type.type_guard self.type_guard: mypy.types.Type | None = None + @property + def fullname(self) -> str: + return self._fullname + + @fullname.setter + def fullname(self, v: str) -> None: + self._fullname = v + class NameExpr(RefExpr): """Name expression @@ -2806,7 +2818,7 @@ class is generic then it will be a type constructor of higher kind. "self_type", ) - _fullname: Bogus[str] # Fully qualified name + _fullname: str # Fully qualified name # Fully qualified name for the module this type was defined in. This # information is also in the fullname, but is harder to extract in the # case of nested class definitions. @@ -3023,7 +3035,7 @@ def name(self) -> str: return self.defn.name @property - def fullname(self) -> Bogus[str]: + def fullname(self) -> str: return self._fullname def is_generic(self) -> bool: @@ -3739,11 +3751,7 @@ def serialize(self, prefix: str, name: str) -> JsonDict: if prefix is not None: fullname = self.node.fullname if ( - # See the comment above SymbolNode.fullname -- fullname can often be None, - # but for complex reasons it's annotated as being `Bogus[str]` instead of `str | None`, - # meaning mypy erroneously thinks the `fullname is not None` check here is redundant - fullname is not None # type: ignore[redundant-expr] - and "." in fullname + "." in fullname and fullname != prefix + "." + name and not (isinstance(self.node, Var) and self.node.from_module_getattr) ): diff --git a/mypy/partially_defined.py b/mypy/partially_defined.py index c63c62c3e393..9a58df04371f 100644 --- a/mypy/partially_defined.py +++ b/mypy/partially_defined.py @@ -287,7 +287,7 @@ def is_undefined(self, name: str) -> bool: def refers_to_builtin(o: RefExpr) -> bool: - return o.fullname is not None and o.fullname.startswith("builtins.") + return o.fullname.startswith("builtins.") class Loop: diff --git a/mypy/semanal.py b/mypy/semanal.py index aee355d7880d..acc485a609e0 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3022,13 +3022,13 @@ def analyze_lvalues(self, s: AssignmentStmt) -> None: def apply_dynamic_class_hook(self, s: AssignmentStmt) -> None: if not isinstance(s.rvalue, CallExpr): return - fname = None + fname = "" call = s.rvalue while True: if isinstance(call.callee, RefExpr): fname = call.callee.fullname # check if method call - if fname is None and isinstance(call.callee, MemberExpr): + if not fname and isinstance(call.callee, MemberExpr): callee_expr = call.callee.expr if isinstance(callee_expr, RefExpr) and callee_expr.fullname: method_name = call.callee.name @@ -4624,7 +4624,7 @@ def bind_name_expr(self, expr: NameExpr, sym: SymbolTableNode) -> None: else: expr.kind = sym.kind expr.node = sym.node - expr.fullname = sym.fullname + expr.fullname = sym.fullname or "" def visit_super_expr(self, expr: SuperExpr) -> None: if not self.type and not expr.call.args: @@ -4849,7 +4849,7 @@ def visit_member_expr(self, expr: MemberExpr) -> None: self.process_placeholder(expr.name, "attribute", expr) return expr.kind = sym.kind - expr.fullname = sym.fullname + expr.fullname = sym.fullname or "" expr.node = sym.node elif isinstance(base, RefExpr): # This branch handles the case C.bar (or cls.bar or self.bar inside @@ -4881,7 +4881,7 @@ def visit_member_expr(self, expr: MemberExpr) -> None: if not n: return expr.kind = n.kind - expr.fullname = n.fullname + expr.fullname = n.fullname or "" expr.node = n.node def visit_op_expr(self, expr: OpExpr) -> None: @@ -5341,7 +5341,7 @@ def is_overloaded_item(self, node: SymbolNode, statement: Statement) -> bool: return False def is_defined_in_current_module(self, fullname: str | None) -> bool: - if fullname is None: + if not fullname: return False return module_prefix(self.modules, fullname) == self.cur_mod_id diff --git a/mypy/server/aststrip.py b/mypy/server/aststrip.py index b0666f8e1ff4..05af6a3d53a1 100644 --- a/mypy/server/aststrip.py +++ b/mypy/server/aststrip.py @@ -230,7 +230,7 @@ def visit_op_expr(self, node: OpExpr) -> None: def strip_ref_expr(self, node: RefExpr) -> None: node.kind = None node.node = None - node.fullname = None + node.fullname = "" node.is_new_def = False node.is_inferred_def = False diff --git a/mypy/server/deps.py b/mypy/server/deps.py index eb40737061bf..63038a90796f 100644 --- a/mypy/server/deps.py +++ b/mypy/server/deps.py @@ -289,13 +289,9 @@ def visit_decorator(self, o: Decorator) -> None: # all call sites, making them all `Any`. for d in o.decorators: tname: str | None = None - if isinstance(d, RefExpr) and d.fullname is not None: + if isinstance(d, RefExpr) and d.fullname: tname = d.fullname - if ( - isinstance(d, CallExpr) - and isinstance(d.callee, RefExpr) - and d.callee.fullname is not None - ): + if isinstance(d, CallExpr) and isinstance(d.callee, RefExpr) and d.callee.fullname: tname = d.callee.fullname if tname is not None: self.add_dependency(make_trigger(tname), make_trigger(o.func.fullname)) @@ -500,7 +496,7 @@ def visit_assignment_stmt(self, o: AssignmentStmt) -> None: if ( isinstance(rvalue, CallExpr) and isinstance(rvalue.callee, RefExpr) - and rvalue.callee.fullname is not None + and rvalue.callee.fullname ): fname: str | None = None if isinstance(rvalue.callee.node, TypeInfo): @@ -638,7 +634,7 @@ def visit_del_stmt(self, o: DelStmt) -> None: # Expressions def process_global_ref_expr(self, o: RefExpr) -> None: - if o.fullname is not None: + if o.fullname: self.add_dependency(make_trigger(o.fullname)) # If this is a reference to a type, generate a dependency to its diff --git a/mypy/strconv.py b/mypy/strconv.py index 861a7c9b7fa0..b2e9da5dbf6a 100644 --- a/mypy/strconv.py +++ b/mypy/strconv.py @@ -367,7 +367,7 @@ def pretty_name( id = "" if isinstance(target_node, mypy.nodes.MypyFile) and name == fullname: n += id - elif kind == mypy.nodes.GDEF or (fullname != name and fullname is not None): + elif kind == mypy.nodes.GDEF or (fullname != name and fullname): # Append fully qualified name for global references. n += f" [{fullname}{id}]" elif kind == mypy.nodes.LDEF: diff --git a/mypy/stubtest.py b/mypy/stubtest.py index bfd8e2b9c81a..774f03cbbdd0 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -1147,7 +1147,7 @@ def apply_decorator_to_funcitem( ) -> nodes.FuncItem | None: if not isinstance(decorator, nodes.RefExpr): return None - if decorator.fullname is None: + if not decorator.fullname: # Happens with namedtuple return None if ( diff --git a/mypy/test/testsemanal.py b/mypy/test/testsemanal.py index 6cfd53f09beb..71ebc43df8c2 100644 --- a/mypy/test/testsemanal.py +++ b/mypy/test/testsemanal.py @@ -202,7 +202,7 @@ def run_case(self, testcase: DataDrivenTestCase) -> None: for f in result.files.values(): for n in f.names.values(): if isinstance(n.node, TypeInfo): - assert n.fullname is not None + assert n.fullname typeinfos[n.fullname] = n.node # The output is the symbol table converted into a string. diff --git a/mypy/typeanal.py b/mypy/typeanal.py index df74344fb392..07720afeff88 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -1123,7 +1123,7 @@ def visit_type_type(self, t: TypeType) -> Type: return TypeType.make_normalized(self.anal_type(t.item), line=t.line) def visit_placeholder_type(self, t: PlaceholderType) -> Type: - n = None if t.fullname is None else self.api.lookup_fully_qualified(t.fullname) + n = None if not t.fullname else self.api.lookup_fully_qualified(t.fullname) if not n or isinstance(n.node, PlaceholderNode): self.api.defer() # Still incomplete return t diff --git a/mypyc/irbuild/builder.py b/mypyc/irbuild/builder.py index c24207ac64ec..a43002d3fe3e 100644 --- a/mypyc/irbuild/builder.py +++ b/mypyc/irbuild/builder.py @@ -1001,7 +1001,7 @@ def call_refexpr_with_args( ) -> Value: # Handle data-driven special-cased primitive call ops. - if callee.fullname is not None and expr.arg_kinds == [ARG_POS] * len(arg_values): + if callee.fullname and expr.arg_kinds == [ARG_POS] * len(arg_values): call_c_ops_candidates = function_ops.get(callee.fullname, []) target = self.builder.matching_call_c( call_c_ops_candidates, arg_values, expr.line, self.node_type(expr) @@ -1026,7 +1026,7 @@ def call_refexpr_with_args( callee_node = callee_node.func if ( callee_node is not None - and callee.fullname is not None + and callee.fullname and callee_node in self.mapper.func_to_decl and all(kind in (ARG_POS, ARG_NAMED) for kind in expr.arg_kinds) ): diff --git a/test-data/unit/plugins/customentry.py b/test-data/unit/plugins/customentry.py index f8b86c33dcfc..b3dacfd4cf44 100644 --- a/test-data/unit/plugins/customentry.py +++ b/test-data/unit/plugins/customentry.py @@ -4,7 +4,7 @@ class MyPlugin(Plugin): def get_function_hook(self, fullname): if fullname == '__main__.f': return my_hook - assert fullname is not None + assert fullname return None def my_hook(ctx): From 9ea5f03c5cdebb4a341626d5354cbb6ce333ba26 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 13 Jan 2023 10:37:35 +0000 Subject: [PATCH 2/2] Minor tweaks --- mypy/checkexpr.py | 4 ++-- mypy/server/deps.py | 2 +- mypy/tvar_scope.py | 2 +- mypyc/irbuild/builder.py | 2 +- mypyc/irbuild/function.py | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 395645b6bf25..1c25b8ea7a12 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -536,7 +536,7 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) -> # get_method_hook() and get_method_signature_hook() will # be invoked for these. if ( - fullname is None + not fullname and isinstance(e.callee, MemberExpr) and self.chk.has_type(e.callee.expr) ): @@ -605,7 +605,7 @@ def method_fullname(self, object_type: Type, method_name: str) -> str | None: elif isinstance(object_type, TupleType): type_name = tuple_fallback(object_type).type.fullname - if type_name is not None: + if type_name: return f"{type_name}.{method_name}" else: return None diff --git a/mypy/server/deps.py b/mypy/server/deps.py index 63038a90796f..50b66b70b8aa 100644 --- a/mypy/server/deps.py +++ b/mypy/server/deps.py @@ -506,7 +506,7 @@ def visit_assignment_stmt(self, o: AssignmentStmt) -> None: fname = init.node.fullname else: fname = rvalue.callee.fullname - if fname is None: + if not fname: return for lv in o.lvalues: if isinstance(lv, RefExpr) and lv.fullname and lv.is_new_def: diff --git a/mypy/tvar_scope.py b/mypy/tvar_scope.py index db83768bf68a..9b432d8e68ec 100644 --- a/mypy/tvar_scope.py +++ b/mypy/tvar_scope.py @@ -129,7 +129,7 @@ def bind_existing(self, tvar_def: TypeVarLikeType) -> None: def get_binding(self, item: str | SymbolTableNode) -> TypeVarLikeType | None: fullname = item.fullname if isinstance(item, SymbolTableNode) else item - assert fullname is not None + assert fullname if fullname in self.scope: return self.scope[fullname] elif self.parent is not None: diff --git a/mypyc/irbuild/builder.py b/mypyc/irbuild/builder.py index a43002d3fe3e..f2a70d4e8691 100644 --- a/mypyc/irbuild/builder.py +++ b/mypyc/irbuild/builder.py @@ -1240,7 +1240,7 @@ def load_global(self, expr: NameExpr) -> Value: and isinstance(expr.node, TypeInfo) and not self.is_synthetic_type(expr.node) ): - assert expr.fullname is not None + assert expr.fullname return self.load_native_type_object(expr.fullname) return self.load_global_str(expr.name, expr.line) diff --git a/mypyc/irbuild/function.py b/mypyc/irbuild/function.py index 5447f945db25..523f8c299c2f 100644 --- a/mypyc/irbuild/function.py +++ b/mypyc/irbuild/function.py @@ -789,7 +789,7 @@ def load_type(builder: IRBuilder, typ: TypeInfo, line: int) -> Value: def load_func(builder: IRBuilder, func_name: str, fullname: str | None, line: int) -> Value: - if fullname is not None and not fullname.startswith(builder.current_module): + if fullname and not fullname.startswith(builder.current_module): # we're calling a function in a different module # We can't use load_module_attr_by_fullname here because we need to load the function using