From 1c76fba1bbad478295c2377a38274af52218a9d0 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 21 Aug 2023 20:12:57 +0100 Subject: [PATCH 1/7] add HAS_EVAL_BREAK flag --- Include/internal/pycore_opcode_metadata.h | 36 ++++++++++++----------- Tools/cases_generator/analysis.py | 3 +- Tools/cases_generator/flags.py | 12 ++++---- 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index cc8894ad53933f..8cfbb59f4ba206 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1148,12 +1148,14 @@ enum InstructionFormat { #define HAS_JUMP_FLAG (8) #define HAS_FREE_FLAG (16) #define HAS_LOCAL_FLAG (32) +#define HAS_EVAL_BREAK_FLAG (64) #define OPCODE_HAS_ARG(OP) (_PyOpcode_opcode_metadata[OP].flags & (HAS_ARG_FLAG)) #define OPCODE_HAS_CONST(OP) (_PyOpcode_opcode_metadata[OP].flags & (HAS_CONST_FLAG)) #define OPCODE_HAS_NAME(OP) (_PyOpcode_opcode_metadata[OP].flags & (HAS_NAME_FLAG)) #define OPCODE_HAS_JUMP(OP) (_PyOpcode_opcode_metadata[OP].flags & (HAS_JUMP_FLAG)) #define OPCODE_HAS_FREE(OP) (_PyOpcode_opcode_metadata[OP].flags & (HAS_FREE_FLAG)) #define OPCODE_HAS_LOCAL(OP) (_PyOpcode_opcode_metadata[OP].flags & (HAS_LOCAL_FLAG)) +#define OPCODE_HAS_EVAL_BREAK(OP) (_PyOpcode_opcode_metadata[OP].flags & (HAS_EVAL_BREAK_FLAG)) struct opcode_metadata { bool valid_entry; @@ -1186,8 +1188,8 @@ extern const struct opcode_metadata _PyOpcode_opcode_metadata[OPCODE_METADATA_SI #ifdef NEED_OPCODE_METADATA const struct opcode_metadata _PyOpcode_opcode_metadata[OPCODE_METADATA_SIZE] = { [NOP] = { true, INSTR_FMT_IX, 0 }, - [RESUME] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, - [INSTRUMENTED_RESUME] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, + [RESUME] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, + [INSTRUMENTED_RESUME] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, [LOAD_CLOSURE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG }, [LOAD_FAST_CHECK] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG }, [LOAD_FAST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG }, @@ -1323,10 +1325,10 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[OPCODE_METADATA_SIZE] = { [IMPORT_NAME] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG }, [IMPORT_FROM] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG }, [JUMP_FORWARD] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG }, - [JUMP_BACKWARD] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG }, + [JUMP_BACKWARD] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_EVAL_BREAK_FLAG }, [JUMP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG }, [JUMP_NO_INTERRUPT] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG }, - [ENTER_EXECUTOR] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG }, + [ENTER_EXECUTOR] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_EVAL_BREAK_FLAG }, [POP_JUMP_IF_FALSE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG }, [POP_JUMP_IF_TRUE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG }, [POP_JUMP_IF_NONE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG }, @@ -1360,28 +1362,28 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[OPCODE_METADATA_SIZE] = { [LOAD_ATTR_METHOD_LAZY_DICT] = { true, INSTR_FMT_IBC00000000, HAS_ARG_FLAG }, [KW_NAMES] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_CONST_FLAG }, [INSTRUMENTED_CALL] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, - [CALL] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, + [CALL] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, [CALL_BOUND_METHOD_EXACT_ARGS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, [CALL_PY_EXACT_ARGS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, [CALL_PY_WITH_DEFAULTS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, [CALL_NO_KW_TYPE_1] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_NO_KW_STR_1] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_NO_KW_TUPLE_1] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, + [CALL_NO_KW_STR_1] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, + [CALL_NO_KW_TUPLE_1] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, [CALL_NO_KW_ALLOC_AND_ENTER_INIT] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, [EXIT_INIT_CHECK] = { true, INSTR_FMT_IX, 0 }, - [CALL_BUILTIN_CLASS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_NO_KW_BUILTIN_O] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_NO_KW_BUILTIN_FAST] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_BUILTIN_FAST_WITH_KEYWORDS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, + [CALL_BUILTIN_CLASS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, + [CALL_NO_KW_BUILTIN_O] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, + [CALL_NO_KW_BUILTIN_FAST] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, + [CALL_BUILTIN_FAST_WITH_KEYWORDS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, [CALL_NO_KW_LEN] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, [CALL_NO_KW_ISINSTANCE] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, [CALL_NO_KW_LIST_APPEND] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_NO_KW_METHOD_DESCRIPTOR_O] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_NO_KW_METHOD_DESCRIPTOR_NOARGS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, - [CALL_NO_KW_METHOD_DESCRIPTOR_FAST] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG }, + [CALL_NO_KW_METHOD_DESCRIPTOR_O] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, + [CALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, + [CALL_NO_KW_METHOD_DESCRIPTOR_NOARGS] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, + [CALL_NO_KW_METHOD_DESCRIPTOR_FAST] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, [INSTRUMENTED_CALL_FUNCTION_EX] = { true, INSTR_FMT_IX, 0 }, - [CALL_FUNCTION_EX] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, + [CALL_FUNCTION_EX] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, [MAKE_FUNCTION] = { true, INSTR_FMT_IX, 0 }, [SET_FUNCTION_ATTRIBUTE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, [RETURN_GENERATOR] = { true, INSTR_FMT_IX, 0 }, @@ -1394,7 +1396,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[OPCODE_METADATA_SIZE] = { [SWAP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, [INSTRUMENTED_INSTRUCTION] = { true, INSTR_FMT_IX, 0 }, [INSTRUMENTED_JUMP_FORWARD] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, - [INSTRUMENTED_JUMP_BACKWARD] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, + [INSTRUMENTED_JUMP_BACKWARD] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, [INSTRUMENTED_POP_JUMP_IF_TRUE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, [INSTRUMENTED_POP_JUMP_IF_FALSE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, [INSTRUMENTED_POP_JUMP_IF_NONE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, diff --git a/Tools/cases_generator/analysis.py b/Tools/cases_generator/analysis.py index 72fb2d761dbfae..43b19779dc03ca 100644 --- a/Tools/cases_generator/analysis.py +++ b/Tools/cases_generator/analysis.py @@ -381,7 +381,8 @@ def analyze_pseudo(self, pseudo: parsing.Pseudo) -> PseudoInstruction: # Make sure the targets have the same fmt fmts = list(set([t.instr_fmt for t in targets])) assert len(fmts) == 1 - assert len(list(set([t.instr_flags.bitmap() for t in targets]))) == 1 + ignored_flags = ['HAS_EVAL_BREAK_FLAG'] + assert len(list(set([t.instr_flags.bitmap(ignore=ignored_flags) for t in targets]))) == 1 return PseudoInstruction(pseudo.name, targets, fmts[0], targets[0].instr_flags) def analyze_instruction( diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index 536f093c7d6ede..6520a703092ef6 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -15,6 +15,7 @@ class InstructionFlags: HAS_JUMP_FLAG: bool HAS_FREE_FLAG: bool HAS_LOCAL_FLAG: bool + HAS_EVAL_BREAK_FLAG: bool def __post_init__(self) -> None: self.bitmask = {name: (1 << i) for i, name in enumerate(self.names())} @@ -35,13 +36,13 @@ def fromInstruction(instr: parsing.Node) -> "InstructionFlags": HAS_FREE_FLAG=has_free, HAS_LOCAL_FLAG=( variable_used(instr, "GETLOCAL") or variable_used(instr, "SETLOCAL") - ) - and not has_free, + ) and not has_free, + HAS_EVAL_BREAK_FLAG=variable_used(instr, "CHECK_EVAL_BREAKER"), ) @staticmethod def newEmpty() -> "InstructionFlags": - return InstructionFlags(False, False, False, False, False, False) + return InstructionFlags(False, False, False, False, False, False, False) def add(self, other: "InstructionFlags") -> None: for name, value in dataclasses.asdict(other).items(): @@ -53,10 +54,11 @@ def names(self, value: bool | None = None) -> list[str]: return list(dataclasses.asdict(self).keys()) return [n for n, v in dataclasses.asdict(self).items() if v == value] - def bitmap(self) -> int: + def bitmap(self, ignore: list[str] = []) -> int: flags = 0 + assert all(hasattr(self, name) for name in ignore) for name in self.names(): - if getattr(self, name): + if getattr(self, name) and name not in ignore: flags |= self.bitmask[name] return flags From 5aa766b681d1a56645af03636f5b04822dc07972 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Wed, 23 Aug 2023 19:41:29 +0100 Subject: [PATCH 2/7] whitespace --- Tools/cases_generator/flags.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index 6520a703092ef6..f006cf5cbe17f9 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -36,7 +36,8 @@ def fromInstruction(instr: parsing.Node) -> "InstructionFlags": HAS_FREE_FLAG=has_free, HAS_LOCAL_FLAG=( variable_used(instr, "GETLOCAL") or variable_used(instr, "SETLOCAL") - ) and not has_free, + ) + and not has_free, HAS_EVAL_BREAK_FLAG=variable_used(instr, "CHECK_EVAL_BREAKER"), ) From 9caeedc9c522a9f63e99c90b887ec0c64a41f0fa Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 25 Aug 2023 11:34:46 +0100 Subject: [PATCH 3/7] tuple instead of list --- Tools/cases_generator/analysis.py | 2 +- Tools/cases_generator/flags.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/cases_generator/analysis.py b/Tools/cases_generator/analysis.py index 43b19779dc03ca..38da0cd92ad8e6 100644 --- a/Tools/cases_generator/analysis.py +++ b/Tools/cases_generator/analysis.py @@ -381,7 +381,7 @@ def analyze_pseudo(self, pseudo: parsing.Pseudo) -> PseudoInstruction: # Make sure the targets have the same fmt fmts = list(set([t.instr_fmt for t in targets])) assert len(fmts) == 1 - ignored_flags = ['HAS_EVAL_BREAK_FLAG'] + ignored_flags = ('HAS_EVAL_BREAK_FLAG',) assert len(list(set([t.instr_flags.bitmap(ignore=ignored_flags) for t in targets]))) == 1 return PseudoInstruction(pseudo.name, targets, fmts[0], targets[0].instr_flags) diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index f006cf5cbe17f9..3fc4995a242d2c 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -55,7 +55,7 @@ def names(self, value: bool | None = None) -> list[str]: return list(dataclasses.asdict(self).keys()) return [n for n, v in dataclasses.asdict(self).items() if v == value] - def bitmap(self, ignore: list[str] = []) -> int: + def bitmap(self, ignore: tuple[str] = ()) -> int: flags = 0 assert all(hasattr(self, name) for name in ignore) for name in self.names(): From e64d5f84078b64af8d40950a52f331b5f3c78d63 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 25 Aug 2023 11:43:06 +0100 Subject: [PATCH 4/7] ok, I finally installed mypy --- Tools/cases_generator/flags.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index 3fc4995a242d2c..a1011ab1cdc574 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -55,8 +55,9 @@ def names(self, value: bool | None = None) -> list[str]: return list(dataclasses.asdict(self).keys()) return [n for n, v in dataclasses.asdict(self).items() if v == value] - def bitmap(self, ignore: tuple[str] = ()) -> int: + def bitmap(self, ignore: tuple[str] | None = None) -> int: flags = 0 + ignore = ignore or () assert all(hasattr(self, name) for name in ignore) for name in self.names(): if getattr(self, name) and name not in ignore: From 98141a4b03e1c4c28d322136ea97fed3186b6828 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 25 Aug 2023 12:39:19 +0100 Subject: [PATCH 5/7] simplify type declaration --- Tools/cases_generator/flags.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index a1011ab1cdc574..349234c83b11d6 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -55,9 +55,8 @@ def names(self, value: bool | None = None) -> list[str]: return list(dataclasses.asdict(self).keys()) return [n for n, v in dataclasses.asdict(self).items() if v == value] - def bitmap(self, ignore: tuple[str] | None = None) -> int: + def bitmap(self, ignore: tuple = ()) -> int: flags = 0 - ignore = ignore or () assert all(hasattr(self, name) for name in ignore) for name in self.names(): if getattr(self, name) and name not in ignore: From 7384ff07600f2117fe3d075e46a67e08335437b2 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 25 Aug 2023 13:49:07 +0100 Subject: [PATCH 6/7] tuple --> AbstractSet --- Tools/cases_generator/analysis.py | 2 +- Tools/cases_generator/flags.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Tools/cases_generator/analysis.py b/Tools/cases_generator/analysis.py index 38da0cd92ad8e6..d4ad654eb47683 100644 --- a/Tools/cases_generator/analysis.py +++ b/Tools/cases_generator/analysis.py @@ -381,7 +381,7 @@ def analyze_pseudo(self, pseudo: parsing.Pseudo) -> PseudoInstruction: # Make sure the targets have the same fmt fmts = list(set([t.instr_fmt for t in targets])) assert len(fmts) == 1 - ignored_flags = ('HAS_EVAL_BREAK_FLAG',) + ignored_flags = set(('HAS_EVAL_BREAK_FLAG',)) assert len(list(set([t.instr_flags.bitmap(ignore=ignored_flags) for t in targets]))) == 1 return PseudoInstruction(pseudo.name, targets, fmts[0], targets[0].instr_flags) diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index 349234c83b11d6..30b764a575fb9b 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -3,6 +3,7 @@ from formatting import Formatter import lexer as lx import parsing +from typing import AbstractSet @dataclasses.dataclass @@ -55,7 +56,7 @@ def names(self, value: bool | None = None) -> list[str]: return list(dataclasses.asdict(self).keys()) return [n for n, v in dataclasses.asdict(self).items() if v == value] - def bitmap(self, ignore: tuple = ()) -> int: + def bitmap(self, ignore: AbstractSet[str] = set()) -> int: flags = 0 assert all(hasattr(self, name) for name in ignore) for name in self.names(): From 4a3f908e59772bc0f0c03cf8f13c325147441b8e Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Fri, 25 Aug 2023 18:52:13 +0100 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Guido van Rossum --- Tools/cases_generator/analysis.py | 4 ++-- Tools/cases_generator/flags.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/cases_generator/analysis.py b/Tools/cases_generator/analysis.py index d4ad654eb47683..b3c3df1378b943 100644 --- a/Tools/cases_generator/analysis.py +++ b/Tools/cases_generator/analysis.py @@ -381,8 +381,8 @@ def analyze_pseudo(self, pseudo: parsing.Pseudo) -> PseudoInstruction: # Make sure the targets have the same fmt fmts = list(set([t.instr_fmt for t in targets])) assert len(fmts) == 1 - ignored_flags = set(('HAS_EVAL_BREAK_FLAG',)) - assert len(list(set([t.instr_flags.bitmap(ignore=ignored_flags) for t in targets]))) == 1 + ignored_flags = {'HAS_EVAL_BREAK_FLAG'} + assert len({t.instr_flags.bitmap(ignore=ignored_flags) for t in targets}) == 1 return PseudoInstruction(pseudo.name, targets, fmts[0], targets[0].instr_flags) def analyze_instruction( diff --git a/Tools/cases_generator/flags.py b/Tools/cases_generator/flags.py index 30b764a575fb9b..193e7ff0d9ccfb 100644 --- a/Tools/cases_generator/flags.py +++ b/Tools/cases_generator/flags.py @@ -56,7 +56,7 @@ def names(self, value: bool | None = None) -> list[str]: return list(dataclasses.asdict(self).keys()) return [n for n, v in dataclasses.asdict(self).items() if v == value] - def bitmap(self, ignore: AbstractSet[str] = set()) -> int: + def bitmap(self, ignore: AbstractSet[str] = frozenset()) -> int: flags = 0 assert all(hasattr(self, name) for name in ignore) for name in self.names():