From 6b8f9c28f3c2baed70ba021ad1f38e63fbade565 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Fri, 14 Oct 2022 09:16:51 -0700 Subject: [PATCH 01/10] keep target labels when debugging, but don't warn about lack of use --- Python/ceval.c | 4 ++++ configure | 2 +- configure.ac | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index c08c794005d1ab..755e417f6b73e8 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -679,7 +679,11 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) #define TARGET(op) TARGET_##op: INSTRUCTION_START(op); #define DISPATCH_GOTO() goto *opcode_targets[opcode] #else +#ifdef Py_DEBUG +#define TARGET(op) case op: TARGET_##op: INSTRUCTION_START(op); +#else #define TARGET(op) case op: INSTRUCTION_START(op); +#endif #define DISPATCH_GOTO() goto dispatch_opcode #endif diff --git a/configure b/configure index 0e9f72faa7b120..3dcacbb33e63da 100755 --- a/configure +++ b/configure @@ -8525,7 +8525,7 @@ then case $ac_cv_prog_cc_g in yes) if test "$Py_DEBUG" = 'true' ; then - OPT="-g $PYDEBUG_CFLAGS -Wall" + OPT="-g $PYDEBUG_CFLAGS -Wall -Wno-unused-label" else OPT="-g $WRAP -O3 -Wall" fi diff --git a/configure.ac b/configure.ac index a4f3b0ab84664a..8bc4d884546916 100644 --- a/configure.ac +++ b/configure.ac @@ -2129,7 +2129,7 @@ then case $ac_cv_prog_cc_g in yes) if test "$Py_DEBUG" = 'true' ; then - OPT="-g $PYDEBUG_CFLAGS -Wall" + OPT="-g $PYDEBUG_CFLAGS -Wall -Wno-unused-label" else OPT="-g $WRAP -O3 -Wall" fi From 3abeec308f5661d48bbe4d288d5b7105893d567c Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Fri, 14 Oct 2022 09:22:27 -0700 Subject: [PATCH 02/10] NEWS blurb --- .../next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst diff --git a/Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst b/Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst new file mode 100644 index 00000000000000..fae586c9fb6523 --- /dev/null +++ b/Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst @@ -0,0 +1,3 @@ +When ``Py_DEBUG`` is enabled, ``Python/ceval.c`` is compiled with target +labels, even if ``USE_COMPUTED_GOTOS`` was disabled. This allows +breakpoints to be set at those labels in (for instance) ``gdb``. From 4071c27f72ced7724ade0c03f9f913dcbf34258b Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Sun, 16 Oct 2022 12:49:52 -0700 Subject: [PATCH 03/10] get the issue number right --- ...9.BsnQ4k.rst => 2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/Build/{2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst => 2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst} (100%) diff --git a/Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst b/Misc/NEWS.d/next/Build/2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst similarity index 100% rename from Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst rename to Misc/NEWS.d/next/Build/2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst From 13bd5fa9d4f2c0120c248a82b120632551406adc Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Fri, 14 Oct 2022 09:16:51 -0700 Subject: [PATCH 04/10] keep target labels when debugging, but don't warn about lack of use --- Python/ceval.c | 4 ++++ configure | 2 +- configure.ac | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 100aa3d727e468..388978197aa048 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -679,7 +679,11 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) #define TARGET(op) TARGET_##op: INSTRUCTION_START(op); #define DISPATCH_GOTO() goto *opcode_targets[opcode] #else +#ifdef Py_DEBUG +#define TARGET(op) case op: TARGET_##op: INSTRUCTION_START(op); +#else #define TARGET(op) case op: INSTRUCTION_START(op); +#endif #define DISPATCH_GOTO() goto dispatch_opcode #endif diff --git a/configure b/configure index 953c558d6048d4..e757400150bcfb 100755 --- a/configure +++ b/configure @@ -8525,7 +8525,7 @@ then case $ac_cv_prog_cc_g in yes) if test "$Py_DEBUG" = 'true' ; then - OPT="-g $PYDEBUG_CFLAGS -Wall" + OPT="-g $PYDEBUG_CFLAGS -Wall -Wno-unused-label" else OPT="-g $WRAP -O3 -Wall" fi diff --git a/configure.ac b/configure.ac index 210ce3292cfdf7..4704669e700ca7 100644 --- a/configure.ac +++ b/configure.ac @@ -2129,7 +2129,7 @@ then case $ac_cv_prog_cc_g in yes) if test "$Py_DEBUG" = 'true' ; then - OPT="-g $PYDEBUG_CFLAGS -Wall" + OPT="-g $PYDEBUG_CFLAGS -Wall -Wno-unused-label" else OPT="-g $WRAP -O3 -Wall" fi From f023acc2630ba3377ddeafe2057fd5e695310a42 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Fri, 14 Oct 2022 09:22:27 -0700 Subject: [PATCH 05/10] NEWS blurb --- .../next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst diff --git a/Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst b/Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst new file mode 100644 index 00000000000000..fae586c9fb6523 --- /dev/null +++ b/Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst @@ -0,0 +1,3 @@ +When ``Py_DEBUG`` is enabled, ``Python/ceval.c`` is compiled with target +labels, even if ``USE_COMPUTED_GOTOS`` was disabled. This allows +breakpoints to be set at those labels in (for instance) ``gdb``. From cb638e3acc9ac671cf4d2751bfabf957ed0409b2 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Sun, 16 Oct 2022 12:49:52 -0700 Subject: [PATCH 06/10] get the issue number right --- ...9.BsnQ4k.rst => 2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/Build/{2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst => 2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst} (100%) diff --git a/Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst b/Misc/NEWS.d/next/Build/2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst similarity index 100% rename from Misc/NEWS.d/next/Build/2022-10-14-09-22-06.gh-issue-25949.BsnQ4k.rst rename to Misc/NEWS.d/next/Build/2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst From c76b9cd6bf5baa968c67f041356fe54d457b8db8 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Thu, 3 Nov 2022 14:56:09 -0500 Subject: [PATCH 07/10] more fine-grained warning suppression --- ...2-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst | 6 ++-- Python/ceval.c | 30 ++++++++++++++----- configure | 2 +- configure.ac | 2 +- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/Misc/NEWS.d/next/Build/2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst b/Misc/NEWS.d/next/Build/2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst index fae586c9fb6523..5f32091739a282 100644 --- a/Misc/NEWS.d/next/Build/2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst +++ b/Misc/NEWS.d/next/Build/2022-10-16-12-49-24.gh-issue-88226.BsnQ4k.rst @@ -1,3 +1,3 @@ -When ``Py_DEBUG`` is enabled, ``Python/ceval.c`` is compiled with target -labels, even if ``USE_COMPUTED_GOTOS`` was disabled. This allows -breakpoints to be set at those labels in (for instance) ``gdb``. +Always define ``TARGET_*`` labels in ``Python/ceval.c``, even if +``USE_COMPUTED_GOTOS`` is disabled. This allows breakpoints to be +set at those labels in (for instance) ``gdb``. diff --git a/Python/ceval.c b/Python/ceval.c index 388978197aa048..92ccbe5997f406 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -596,6 +596,7 @@ PyEval_EvalFrame(PyFrameObject *f) return _PyEval_EvalFrame(tstate, f->f_frame, 0); } + PyObject * PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) { @@ -676,15 +677,11 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) #endif #if USE_COMPUTED_GOTOS -#define TARGET(op) TARGET_##op: INSTRUCTION_START(op); -#define DISPATCH_GOTO() goto *opcode_targets[opcode] -#else -#ifdef Py_DEBUG -#define TARGET(op) case op: TARGET_##op: INSTRUCTION_START(op); +# define TARGET(op) TARGET_##op: INSTRUCTION_START(op); +# define DISPATCH_GOTO() goto *opcode_targets[opcode] #else -#define TARGET(op) case op: INSTRUCTION_START(op); -#endif -#define DISPATCH_GOTO() goto dispatch_opcode +# define TARGET(op) case op: TARGET_##op: INSTRUCTION_START(op); +# define DISPATCH_GOTO() goto dispatch_opcode #endif /* PRE_DISPATCH_GOTO() does lltrace if enabled. Normally a no-op */ @@ -1025,6 +1022,18 @@ typedef struct { #define KWNAMES_LEN() \ (call_shape.kwnames == NULL ? 0 : ((int)PyTuple_GET_SIZE(call_shape.kwnames))) +/* Disable unused label warnings. They are handy for debugging, even + if computed gotos aren't used. */ + +/* TBD - what about other compilers? */ +#ifdef __GNUC__ +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wunused-label" +#else /* MS_WINDOWS ? */ +# pragma warning(push) +# pragma warning(disable:4102) +#endif + PyObject* _Py_HOT_FUNCTION _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int throwflag) { @@ -5239,6 +5248,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int goto error; } +#ifdef __GNUC__ +# pragma GCC diagnostic pop +#else /* MS_WINDOWS ? */ +# pragma warning(pop) +#endif static void format_missing(PyThreadState *tstate, const char *kind, diff --git a/configure b/configure index e757400150bcfb..953c558d6048d4 100755 --- a/configure +++ b/configure @@ -8525,7 +8525,7 @@ then case $ac_cv_prog_cc_g in yes) if test "$Py_DEBUG" = 'true' ; then - OPT="-g $PYDEBUG_CFLAGS -Wall -Wno-unused-label" + OPT="-g $PYDEBUG_CFLAGS -Wall" else OPT="-g $WRAP -O3 -Wall" fi diff --git a/configure.ac b/configure.ac index 4704669e700ca7..210ce3292cfdf7 100644 --- a/configure.ac +++ b/configure.ac @@ -2129,7 +2129,7 @@ then case $ac_cv_prog_cc_g in yes) if test "$Py_DEBUG" = 'true' ; then - OPT="-g $PYDEBUG_CFLAGS -Wall -Wno-unused-label" + OPT="-g $PYDEBUG_CFLAGS -Wall" else OPT="-g $WRAP -O3 -Wall" fi From 564aeff4bfdec5fa0d5b093de54e5e4da12812c0 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Thu, 3 Nov 2022 14:59:32 -0500 Subject: [PATCH 08/10] bike shedding --- Python/ceval.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 92ccbe5997f406..f98085913cd908 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -596,7 +596,6 @@ PyEval_EvalFrame(PyFrameObject *f) return _PyEval_EvalFrame(tstate, f->f_frame, 0); } - PyObject * PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) { @@ -1029,7 +1028,7 @@ typedef struct { #ifdef __GNUC__ # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wunused-label" -#else /* MS_WINDOWS ? */ +#else /* MS_WINDOWS */ # pragma warning(push) # pragma warning(disable:4102) #endif @@ -5250,7 +5249,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int } #ifdef __GNUC__ # pragma GCC diagnostic pop -#else /* MS_WINDOWS ? */ +#else /* MS_WINDOWS */ # pragma warning(pop) #endif From 5cfe3aafaab77e65f08d4d662b868df703b321fd Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Thu, 3 Nov 2022 15:32:30 -0500 Subject: [PATCH 09/10] Apply suggestions from code review Co-authored-by: Eryk Sun --- Python/ceval.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 6b263df083cbe1..3fd02d8cb18e44 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1025,10 +1025,10 @@ typedef struct { if computed gotos aren't used. */ /* TBD - what about other compilers? */ -#ifdef __GNUC__ +#if defined(__GNUC__) # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wunused-label" -#else /* MS_WINDOWS */ +#elif defined(_MSC_VER) /* MS_WINDOWS */ # pragma warning(push) # pragma warning(disable:4102) #endif @@ -1386,9 +1386,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int goto error; } -#ifdef __GNUC__ +#if defined(__GNUC__) # pragma GCC diagnostic pop -#else /* MS_WINDOWS */ +#elif defined(_MSC_VER) /* MS_WINDOWS */ # pragma warning(pop) #endif From bf13503f95ec4ddafa1f61635cee2ab4f4e91b2e Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Fri, 4 Nov 2022 06:42:50 -0500 Subject: [PATCH 10/10] how did these sneak back in? --- configure | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 7af79ae8736a38..edd3771784c76f 100755 --- a/configure +++ b/configure @@ -8413,7 +8413,7 @@ then case $ac_cv_prog_cc_g in yes) if test "$Py_DEBUG" = 'true' ; then - OPT="-g $PYDEBUG_CFLAGS -Wall -Wno-unused-label" + OPT="-g $PYDEBUG_CFLAGS -Wall" else OPT="-g $WRAP -O3 -Wall" fi diff --git a/configure.ac b/configure.ac index 63c9c9a625f2b7..0ca5e3fcbf5496 100644 --- a/configure.ac +++ b/configure.ac @@ -2118,7 +2118,7 @@ then case $ac_cv_prog_cc_g in yes) if test "$Py_DEBUG" = 'true' ; then - OPT="-g $PYDEBUG_CFLAGS -Wall -Wno-unused-label" + OPT="-g $PYDEBUG_CFLAGS -Wall" else OPT="-g $WRAP -O3 -Wall" fi