From 443a7641f4fe17381c3c9084e5d615e863859d38 Mon Sep 17 00:00:00 2001 From: Jan Kuehle Date: Thu, 26 Sep 2024 08:49:38 -0700 Subject: [PATCH] Sort blocks by line number of the first opcode Blocks are sorted by number of predecessors. Previously the index of the first opcode, i.e. the order in the bytecode, served as a tie breaker. In Python 3.12 the added test case fails with: dummy_input_file:10:3: error: in : None [assert-type] Expected: Optional[str] Actual: None assert_type(err, str|None) ~~~~~~~~~~~~~~~~~~~~~~~~~~ Python 3.12 emits the bytecode for the `except` block last for some reason. Because of the while loop, every block is a predecessor of every other block, and the predecessor-based sorting fails. It falls back to the tie breaker and pytype executes the `except` block last, i.e. after `assert_type`. By the time pytype executes `assert_type` it don't know about the possible `str` type. This change uses the line number of the first opcode as a tie breaker first and then falls back to the index. PiperOrigin-RevId: 679166584 --- pytype/blocks/blocks.py | 6 ++++++ pytype/tests/test_flow2.py | 14 ++++++++++++++ pytype/typegraph/cfg_utils.py | 17 +++++++++++------ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/pytype/blocks/blocks.py b/pytype/blocks/blocks.py index 9282c0b63..5a2e71614 100644 --- a/pytype/blocks/blocks.py +++ b/pytype/blocks/blocks.py @@ -78,6 +78,12 @@ def __getitem__(self, index_or_slice): def __iter__(self): return self.code.__iter__() + def __lt__(self, other: Self) -> bool: + return (self.code[0].line, self.id) < (other.code[0].line, other.id) + + def __gt__(self, other: Self) -> bool: + return (self.code[0].line, self.id) > (other.code[0].line, other.id) + class OrderedCode: """Code object which knows about instruction ordering. diff --git a/pytype/tests/test_flow2.py b/pytype/tests/test_flow2.py index 018c7b871..29aed9be7 100644 --- a/pytype/tests/test_flow2.py +++ b/pytype/tests/test_flow2.py @@ -95,6 +95,20 @@ def f() -> int: f() """) + def test_try_in_loop(self): + self.Check(""" + def may_fail(): + pass + + while True: + err = None + try: + may_fail() + except ValueError as e: + err = str(e) + assert_type(err, str|None) + """) + if __name__ == "__main__": test_base.main() diff --git a/pytype/typegraph/cfg_utils.py b/pytype/typegraph/cfg_utils.py index 1a33ec0c4..03562c398 100644 --- a/pytype/typegraph/cfg_utils.py +++ b/pytype/typegraph/cfg_utils.py @@ -272,7 +272,12 @@ def compute_predecessors( class OrderableNode(PredecessorNode, Protocol): - id: int + + def __lt__(self, other: "OrderableNode", /) -> bool: + ... + + def __gt__(self, other: "OrderableNode", /) -> bool: + ... _OrderableNode = TypeVar("_OrderableNode", bound=OrderableNode) @@ -287,8 +292,9 @@ def order_nodes(nodes: Sequence[_OrderableNode]) -> list[_OrderableNode]: process both the branches before that node). Args: - nodes: A list of nodes or blocks. They have two attributes: "id" (an int to - enable deterministic sorting) and "outgoing" (a list of nodes). + nodes: A list of nodes or blocks. They have an attribute `outgoing` (a list + of nodes) and define rich comparisons method `__lt__` and `__gt__` to + enable deterministic sorting. Returns: A list of nodes in the proper order. @@ -308,9 +314,8 @@ def order_nodes(nodes: Sequence[_OrderableNode]) -> list[_OrderableNode]: while queue: # Find node with minimum amount of predecessors that's connected to a node # we already processed. - _, _, node = min( - (len(predecessors), node.id, node) - for node, predecessors in queue.items() + _, node = min( + (len(predecessors), node) for node, predecessors in queue.items() ) del queue[node] if node in seen: