From 527f48c288cba574e5470bd9925c0f339f6a0f5c Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 14:24:57 +0000 Subject: [PATCH 01/10] WIP --- mypy/newsemanal/semanal.py | 20 +++++++++++++++----- mypy/newsemanal/semanal_main.py | 2 +- test-data/unit/check-newsemanal.test | 13 +++++++++++++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index b504d96b7919c..831ca4106dba7 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -491,8 +491,14 @@ def file_context(self, file_node: MypyFile, fnam: str, options: Options, del self.options def visit_func_def(self, defn: FuncDef) -> None: - if not defn.is_decorated and not defn.is_overload: - self.add_func_to_symbol_table(defn) + defn.is_conditional = self.block_depth[-1] > 0 + + # Add module-level function to symbol table on first iteration + # (recurse_into_functions == False). Also add nested functions + # always, since they won't be processed as separate targets. + if not self.recurse_into_functions or len(self.function_stack) > 0: + if not defn.is_decorated and not defn.is_overload: + self.add_func_to_symbol_table(defn) if not self.recurse_into_functions: return @@ -515,8 +521,6 @@ def _visit_func_def(self, defn: FuncDef) -> None: self.update_function_type_variables(defn.type, defn) self.function_stack.pop() - defn.is_conditional = self.block_depth[-1] > 0 - if self.is_class_scope(): # Method definition assert self.type is not None @@ -3954,7 +3958,13 @@ def add_symbol_table_node(self, name: str, symbol: SymbolTableNode, and context is not None and not isinstance(existing.node, PlaceholderNode)): if existing.node != symbol.node: - self.name_already_defined(name, context, existing) + if (isinstance(symbol.node, FuncDef) + and self.set_original_def(existing.node, symbol.node)): + # Redefinition of a function. Must keep track of it in the symbol + # table with a dummy name. + names[name + '!'] = symbol + else: + self.name_already_defined(name, context, existing) elif name not in self.missing_names and '*' not in self.missing_names: names[name] = symbol return True diff --git a/mypy/newsemanal/semanal_main.py b/mypy/newsemanal/semanal_main.py index ce8586651ce44..7307fae4db4ba 100644 --- a/mypy/newsemanal/semanal_main.py +++ b/mypy/newsemanal/semanal_main.py @@ -160,7 +160,7 @@ def get_all_leaf_targets(symtable: SymbolTable, """Return all leaf targets in a symbol table (module-level and methods).""" result = [] # type: List[TargetInfo] for name, node in symtable.items(): - new_prefix = prefix + '.' + name + new_prefix = prefix + '.' + name.rstrip('!') if isinstance(node.node, (FuncDef, TypeInfo, OverloadedFuncDef, Decorator)): if node.node.fullname() == new_prefix: if isinstance(node.node, TypeInfo): diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index ec108f9f02523..47416a72fe68e 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1417,3 +1417,16 @@ reveal_type(x) # E: Revealed type is 'Tuple[builtins.int, builtins.int]' [file b.py] import a x = (1, 2) + +[case testNewAnalyzerConditionalFunc] +if int(): + def f(x: int) -> None: + pass + def g(x: int) -> None: + pass +else: + def f(x: int) -> None: + 1() # E: "int" not callable + def g(x: str) -> None: # E: All conditional function variants must have identical signatures + pass +reveal_type(g) # E: Revealed type is 'def (x: builtins.int)' From fde9f36d6df589c4c28d651ed3ba96aad7a21c82 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 15:27:01 +0000 Subject: [PATCH 02/10] More WIP --- mypy/newsemanal/semanal.py | 11 ++++++++--- mypy/test/hacks.py | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 831ca4106dba7..a8b25396315be 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -578,7 +578,7 @@ def prepare_method_signature(self, func: FuncDef, info: TypeInfo) -> None: leading_type = self.class_type(leading_type) func.type = replace_implicit_first_type(functype, leading_type) - def set_original_def(self, previous: Optional[Node], new: FuncDef) -> bool: + def set_original_def(self, previous: Optional[Node], new: Union[FuncDef, Decorator]) -> bool: """If 'new' conditionally redefine 'previous', set 'previous' as original We reject straight redefinitions of functions, as they are usually @@ -587,6 +587,8 @@ def set_original_def(self, previous: Optional[Node], new: FuncDef) -> bool: def f(): ... def f(): ... # Error: 'f' redefined """ + if isinstance(new, Decorator): + new = new.func if isinstance(previous, (FuncDef, Var, Decorator)) and new.is_conditional: new.original_def = previous return True @@ -2793,6 +2795,7 @@ def process_module_assignment(self, lvals: List[Lvalue], rval: Expression, lnode.node = rnode.node def visit_decorator(self, dec: Decorator) -> None: + dec.func.is_conditional = self.block_depth[-1] > 0 if not dec.is_overload: self.add_symbol(dec.name(), dec, dec) dec.func._fullname = self.qualified_name(dec.name()) @@ -3958,11 +3961,13 @@ def add_symbol_table_node(self, name: str, symbol: SymbolTableNode, and context is not None and not isinstance(existing.node, PlaceholderNode)): if existing.node != symbol.node: - if (isinstance(symbol.node, FuncDef) - and self.set_original_def(existing.node, symbol.node)): + if isinstance(symbol.node, (FuncDef, Decorator)): # Redefinition of a function. Must keep track of it in the symbol # table with a dummy name. names[name + '!'] = symbol + if (isinstance(symbol.node, (FuncDef, Decorator)) + and self.set_original_def(existing.node, symbol.node)): + pass else: self.name_already_defined(name, context, existing) elif name not in self.missing_names and '*' not in self.missing_names: diff --git a/mypy/test/hacks.py b/mypy/test/hacks.py index 74a3456761777..37e180b823de1 100644 --- a/mypy/test/hacks.py +++ b/mypy/test/hacks.py @@ -21,7 +21,7 @@ 'check-expressions.test', 'check-fastparse.test', 'check-flags.test', - 'check-functions.test', + #'check-functions.test', 'check-generics.test', 'check-incomplete-fixture.test', 'check-incremental.test', From 6766a7723540b02d8c12fac04c278166922c2fcb Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 15:51:12 +0000 Subject: [PATCH 03/10] Add test case --- test-data/unit/check-newsemanal.test | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 47416a72fe68e..2d21c37a34b64 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1430,3 +1430,19 @@ else: def g(x: str) -> None: # E: All conditional function variants must have identical signatures pass reveal_type(g) # E: Revealed type is 'def (x: builtins.int)' + +[case testNewAnalyzerConditionalDecoratedFunc] +from typing import Callable + +def dec(f: Callable[[int], None]) -> Callable[[str], None]: + pass + +if int(): + from m import f +else: + @dec + def f(x: int) -> None: + 1() # E: "int" not callable +reveal_type(f) # E: Revealed type is 'def (x: builtins.str)' +[file m.py] +def f(x: str) -> None: pass From 8f2bd31a23ba65446e1ca30a8c63b94c992c287f Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 15:53:52 +0000 Subject: [PATCH 04/10] Add test case --- test-data/unit/check-newsemanal.test | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 2d21c37a34b64..61e26f508c8b9 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1431,6 +1431,22 @@ else: pass reveal_type(g) # E: Revealed type is 'def (x: builtins.int)' +[case testNewAnalyzerConditionalFuncDefer] +if int(): + def f(x: A) -> None: + pass + def g(x: A) -> None: + pass +else: + def f(x: A) -> None: + 1() # E: "int" not callable + def g(x: str) -> None: # E: All conditional function variants must have identical signatures + pass + +reveal_type(g) # E: Revealed type is 'def (x: __main__.A)' + +class A: pass + [case testNewAnalyzerConditionalDecoratedFunc] from typing import Callable From 5c1e2f18bdb82a7b0b9a40ff425279f09802c977 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 16:15:55 +0000 Subject: [PATCH 05/10] Update test case to crash --- test-data/unit/check-newsemanal.test | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 61e26f508c8b9..d2d0131ae53cc 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1424,11 +1424,15 @@ if int(): pass def g(x: int) -> None: pass -else: +elif bool(): def f(x: int) -> None: 1() # E: "int" not callable def g(x: str) -> None: # E: All conditional function variants must have identical signatures pass +else: + def f(x: int) -> None: + ''() # E: "int" not callable + reveal_type(g) # E: Revealed type is 'def (x: builtins.int)' [case testNewAnalyzerConditionalFuncDefer] From f900d643d187add8e95cb17633059a49f9742418 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 17:26:11 +0000 Subject: [PATCH 06/10] Use -redefinition[N] name mangling + add test case --- mypy/newsemanal/semanal.py | 27 ++++++++++++++++++++++++--- mypy/newsemanal/semanal_main.py | 5 ++++- test-data/unit/check-newsemanal.test | 12 +++++++++++- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index a8b25396315be..2229b63b19103 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -3962,9 +3962,7 @@ def add_symbol_table_node(self, name: str, symbol: SymbolTableNode, and not isinstance(existing.node, PlaceholderNode)): if existing.node != symbol.node: if isinstance(symbol.node, (FuncDef, Decorator)): - # Redefinition of a function. Must keep track of it in the symbol - # table with a dummy name. - names[name + '!'] = symbol + self.add_func_redefinition(names, name, symbol) if (isinstance(symbol.node, (FuncDef, Decorator)) and self.set_original_def(existing.node, symbol.node)): pass @@ -3975,6 +3973,29 @@ def add_symbol_table_node(self, name: str, symbol: SymbolTableNode, return True return False + def add_func_redefinition(self, names: SymbolTable, name: str, + symbol: SymbolTableNode) -> None: + """Add a symbol table node that reflects a redefinition of a function. + + Redefinitions need to be added to the symbol table so that they can be found + through AST traversal, but they have dummy names of form 'name-redefinition[N]', + where N ranges over 2, 3, ... (omitted for the first redefinition). + """ + i = 1 + while True: + if i == 1: + new_name = '{}-redefinition'.format(name) + else: + new_name = '{}-redefinition{}'.format(name, i) + existing = names.get(new_name) + if existing is None: + names[new_name] = symbol + return + elif existing.node is symbol.node: + # Already there + return + i += 1 + def add_module_symbol(self, id: str, as_id: str, module_public: bool, context: Context, module_hidden: bool = False) -> None: """Add symbol that is a reference to a module object.""" diff --git a/mypy/newsemanal/semanal_main.py b/mypy/newsemanal/semanal_main.py index 7307fae4db4ba..f6e13e6c92110 100644 --- a/mypy/newsemanal/semanal_main.py +++ b/mypy/newsemanal/semanal_main.py @@ -160,7 +160,10 @@ def get_all_leaf_targets(symtable: SymbolTable, """Return all leaf targets in a symbol table (module-level and methods).""" result = [] # type: List[TargetInfo] for name, node in symtable.items(): - new_prefix = prefix + '.' + name.rstrip('!') + shortname = name + if '-redef' in name: + shortname = name.split('-redef')[0] + new_prefix = prefix + '.' + shortname if isinstance(node.node, (FuncDef, TypeInfo, OverloadedFuncDef, Decorator)): if node.node.fullname() == new_prefix: if isinstance(node.node, TypeInfo): diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index d2d0131ae53cc..bbe549965a43f 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1431,7 +1431,7 @@ elif bool(): pass else: def f(x: int) -> None: - ''() # E: "int" not callable + ''() # E: "str" not callable reveal_type(g) # E: Revealed type is 'def (x: builtins.int)' @@ -1466,3 +1466,13 @@ else: reveal_type(f) # E: Revealed type is 'def (x: builtins.str)' [file m.py] def f(x: str) -> None: pass + +[case testNewAnalyzerConditionallyDefineFuncOverVar] +from typing import Callable + +if int(): + f: Callable[[str], None] +else: + def f(x: str) -> None: + ... +reveal_type(f) # E: Revealed type is 'def (builtins.str)' From aa4bc611138622cc83eab384fde219d19e6c7bd2 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 17:40:47 +0000 Subject: [PATCH 07/10] Minor cleanup --- mypy/newsemanal/semanal.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 2229b63b19103..9fafa5acc4687 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -3963,10 +3963,8 @@ def add_symbol_table_node(self, name: str, symbol: SymbolTableNode, if existing.node != symbol.node: if isinstance(symbol.node, (FuncDef, Decorator)): self.add_func_redefinition(names, name, symbol) - if (isinstance(symbol.node, (FuncDef, Decorator)) + if not (isinstance(symbol.node, (FuncDef, Decorator)) and self.set_original_def(existing.node, symbol.node)): - pass - else: self.name_already_defined(name, context, existing) elif name not in self.missing_names and '*' not in self.missing_names: names[name] = symbol From 5be8cc29818ece0e5cf0b62a7d70bdf22ceae300 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 17:42:08 +0000 Subject: [PATCH 08/10] Uncomment line --- mypy/test/hacks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/test/hacks.py b/mypy/test/hacks.py index 37e180b823de1..74a3456761777 100644 --- a/mypy/test/hacks.py +++ b/mypy/test/hacks.py @@ -21,7 +21,7 @@ 'check-expressions.test', 'check-fastparse.test', 'check-flags.test', - #'check-functions.test', + 'check-functions.test', 'check-generics.test', 'check-incomplete-fixture.test', 'check-incremental.test', From 971afc488397567a9844c954e59cb97e5a474dee Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 18 Feb 2019 17:48:12 +0000 Subject: [PATCH 09/10] Add comment --- mypy/newsemanal/semanal_main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mypy/newsemanal/semanal_main.py b/mypy/newsemanal/semanal_main.py index f6e13e6c92110..5244aca0a25fe 100644 --- a/mypy/newsemanal/semanal_main.py +++ b/mypy/newsemanal/semanal_main.py @@ -162,6 +162,7 @@ def get_all_leaf_targets(symtable: SymbolTable, for name, node in symtable.items(): shortname = name if '-redef' in name: + # Restore original name from mangled name of multiply defined function shortname = name.split('-redef')[0] new_prefix = prefix + '.' + shortname if isinstance(node.node, (FuncDef, TypeInfo, OverloadedFuncDef, Decorator)): From baa9c74d21f22a881a2488631a6d206ecc32c295 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 19 Feb 2019 10:45:10 +0000 Subject: [PATCH 10/10] Update based on feedback --- mypy/newsemanal/semanal.py | 7 ++++--- test-data/unit/check-newsemanal.test | 6 ++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 9fafa5acc4687..505b1092ac2e4 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -493,9 +493,10 @@ def file_context(self, file_node: MypyFile, fnam: str, options: Options, def visit_func_def(self, defn: FuncDef) -> None: defn.is_conditional = self.block_depth[-1] > 0 - # Add module-level function to symbol table on first iteration - # (recurse_into_functions == False). Also add nested functions - # always, since they won't be processed as separate targets. + # We don't add module top-level functions to symbol tables + # when we analyze their bodies in the second phase on analysis, + # since they were added in the first phase. Nested functions + # get always added, since they aren't separate targets. if not self.recurse_into_functions or len(self.function_stack) > 0: if not defn.is_decorated and not defn.is_overload: self.add_func_to_symbol_table(defn) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index bbe549965a43f..5368354c6647a 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -1476,3 +1476,9 @@ else: def f(x: str) -> None: ... reveal_type(f) # E: Revealed type is 'def (builtins.str)' + +[case testNewAnalyzerConditionallyDefineFuncOverClass] +class C: + 1() # E: "int" not callable +def C() -> None: # E: Name 'C' already defined on line 1 + ''() # E: "str" not callable