* [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod
@ 2024-06-10 18:13 Tom de Vries
2024-06-10 20:09 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2024-06-10 18:13 UTC (permalink / raw)
To: gdb-patches
I noticed code like this:
...
gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll",
"i", num_to_scroll, nullptr));
...
The nullptr is superfluous, the format string already indicates that there's
only one argument.
OTOH, passing no args does use a nullptr:
...
gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render",
nullptr));
...
Furthermore, choosing the right format string chars can be tricky.
Add non-variadic versions of PyObject_CallMethod that hide these details, such
that we can use the more intuitive:
...
gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll",
num_to_scroll));
...
and:
...
gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render"));
...
Tested on x86_64-linux.
---
gdb/python/py-breakpoint.c | 2 +-
gdb/python/py-disasm.c | 2 +-
gdb/python/py-finishbreakpoint.c | 3 +-
gdb/python/py-framefilter.c | 17 +++++-----
gdb/python/py-tui.c | 13 +++-----
gdb/python/python-internal.h | 53 ++++++++++++++++++++++++++++++++
6 files changed, 69 insertions(+), 21 deletions(-)
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index da74d69d357..d72782e30c6 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1174,7 +1174,7 @@ gdbpy_breakpoint_cond_says_stop (const struct extension_language_defn *extlang,
if (PyObject_HasAttrString (py_bp, stop_func))
{
- gdbpy_ref<> result (PyObject_CallMethod (py_bp, stop_func, NULL));
+ gdbpy_ref<> result (PyObject_CallMethod (py_bp, stop_func));
stop = 1;
if (result != NULL)
diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 9337514acba..e538d1e35f7 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -857,7 +857,7 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff,
overridden by the user. */
gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
"read_memory",
- "I" GDB_PY_LL_ARG, len, offset));
+ len, offset));
/* Handle any exceptions. */
if (result_obj == nullptr)
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index c74a2473a81..8726d6554d2 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -344,8 +344,7 @@ bpfinishpy_out_of_scope (struct finish_breakpoint_object *bpfinish_obj)
if (bpfinish_obj->py_bp.bp->enable_state == bp_enabled
&& PyObject_HasAttrString (py_obj, outofscope_func))
{
- gdbpy_ref<> meth_result (PyObject_CallMethod (py_obj, outofscope_func,
- NULL));
+ gdbpy_ref<> meth_result (PyObject_CallMethod (py_obj, outofscope_func));
if (meth_result == NULL)
gdbpy_print_stack ();
}
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 0cd15977d2f..c38c0219db1 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -59,7 +59,7 @@ extract_sym (PyObject *obj, gdb::unique_xmalloc_ptr<char> *name,
struct symbol **sym, const struct block **sym_block,
const struct language_defn **language)
{
- gdbpy_ref<> result (PyObject_CallMethod (obj, "symbol", NULL));
+ gdbpy_ref<> result (PyObject_CallMethod (obj, "symbol"));
if (result == NULL)
return EXT_LANG_BT_ERROR;
@@ -130,7 +130,7 @@ extract_value (PyObject *obj, struct value **value)
{
if (PyObject_HasAttrString (obj, "value"))
{
- gdbpy_ref<> vresult (PyObject_CallMethod (obj, "value", NULL));
+ gdbpy_ref<> vresult (PyObject_CallMethod (obj, "value"));
if (vresult == NULL)
return EXT_LANG_BT_ERROR;
@@ -264,7 +264,7 @@ get_py_iter_from_func (PyObject *filter, const char *func)
{
if (PyObject_HasAttrString (filter, func))
{
- gdbpy_ref<> result (PyObject_CallMethod (filter, func, NULL));
+ gdbpy_ref<> result (PyObject_CallMethod (filter, func));
if (result != NULL)
{
@@ -790,8 +790,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
/* Get the underlying frame. This is needed to determine GDB
architecture, and also, in the cases of frame variables/arguments to
read them if they returned filter object requires us to do so. */
- gdbpy_ref<> py_inf_frame (PyObject_CallMethod (filter, "inferior_frame",
- NULL));
+ gdbpy_ref<> py_inf_frame (PyObject_CallMethod (filter, "inferior_frame"));
if (py_inf_frame == NULL)
return EXT_LANG_BT_ERROR;
@@ -831,7 +830,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
address printing. */
if (PyObject_HasAttrString (filter, "address"))
{
- gdbpy_ref<> paddr (PyObject_CallMethod (filter, "address", NULL));
+ gdbpy_ref<> paddr (PyObject_CallMethod (filter, "address"));
if (paddr == NULL)
return EXT_LANG_BT_ERROR;
@@ -906,7 +905,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
/* Print frame function name. */
if (PyObject_HasAttrString (filter, "function"))
{
- gdbpy_ref<> py_func (PyObject_CallMethod (filter, "function", NULL));
+ gdbpy_ref<> py_func (PyObject_CallMethod (filter, "function"));
const char *function = NULL;
if (py_func == NULL)
@@ -970,7 +969,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
if (PyObject_HasAttrString (filter, "filename"))
{
- gdbpy_ref<> py_fn (PyObject_CallMethod (filter, "filename", NULL));
+ gdbpy_ref<> py_fn (PyObject_CallMethod (filter, "filename"));
if (py_fn == NULL)
return EXT_LANG_BT_ERROR;
@@ -994,7 +993,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
if (PyObject_HasAttrString (filter, "line"))
{
- gdbpy_ref<> py_line (PyObject_CallMethod (filter, "line", NULL));
+ gdbpy_ref<> py_line (PyObject_CallMethod (filter, "line"));
int line;
if (py_line == NULL)
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 5dcec4bd2b1..29ac7e0f710 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -164,8 +164,7 @@ tui_py_window::~tui_py_window ()
if (m_window != nullptr
&& PyObject_HasAttrString (m_window.get (), "close"))
{
- gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "close",
- nullptr));
+ gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "close"));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -198,8 +197,7 @@ tui_py_window::rerender ()
if (PyObject_HasAttrString (m_window.get (), "render"))
{
- gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render",
- nullptr));
+ gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render"));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -213,7 +211,7 @@ tui_py_window::do_scroll_horizontal (int num_to_scroll)
if (PyObject_HasAttrString (m_window.get (), "hscroll"))
{
gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll",
- "i", num_to_scroll, nullptr));
+ num_to_scroll));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -227,7 +225,7 @@ tui_py_window::do_scroll_vertical (int num_to_scroll)
if (PyObject_HasAttrString (m_window.get (), "vscroll"))
{
gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "vscroll",
- "i", num_to_scroll, nullptr));
+ num_to_scroll));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -249,8 +247,7 @@ tui_py_window::click (int mouse_x, int mouse_y, int mouse_button)
if (PyObject_HasAttrString (m_window.get (), "click"))
{
gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "click",
- "iii", mouse_x, mouse_y,
- mouse_button));
+ mouse_x, mouse_y, mouse_button));
if (result == nullptr)
gdbpy_print_stack ();
}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index d07f239ca65..8cb723b9bf8 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -100,6 +100,12 @@
#define PyEval_ReleaseLock()
#endif
+/* Format string chars as defined here [1], in a readable and intuitive form.
+ [1] https://docs.python.org/3/c-api/arg.html#building-values. */
+
+#define GDB_PY_INT_ARG "i"
+#define GDB_PY_UNSIGNED_INT_ARG "I"
+
/* Python supplies HAVE_LONG_LONG and some `long long' support when it
is available. These defines let us handle the differences more
cleanly.
@@ -166,6 +172,53 @@ gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format,
#undef PyObject_CallMethod
#define PyObject_CallMethod gdb_PyObject_CallMethod
+/* Non-variadic versions of PyObject_CallMethod. */
+
+static inline PyObject *
+PyObject_CallMethod (PyObject *obj, const char *name)
+{
+ return PyObject_CallMethod (obj, name, nullptr);
+}
+
+template<typename T>
+inline PyObject *
+PyObject_CallMethod (PyObject *obj, const char *name, T);
+
+template<>
+inline PyObject *
+PyObject_CallMethod (PyObject *obj, const char *name, int arg1)
+{
+ return PyObject_CallMethod (obj, name, GDB_PY_INT_ARG, arg1);
+}
+
+template<typename T, typename T2>
+inline PyObject *
+PyObject_CallMethod (PyObject *obj, const char *name, T, T2);
+
+template<>
+inline PyObject *
+PyObject_CallMethod (PyObject *obj, const char *name, unsigned int arg1,
+ gdb_py_longest arg2)
+{
+ return PyObject_CallMethod (obj, name,
+ GDB_PY_UNSIGNED_INT_ARG GDB_PY_LL_ARG,
+ arg1, arg2);
+}
+
+template<typename T, typename T2, typename T3>
+inline PyObject *
+PyObject_CallMethod (PyObject *obj, const char *name, T, T2, T3);
+
+template<>
+inline PyObject *
+PyObject_CallMethod (PyObject *obj, const char *name, int arg1, int arg2,
+ int arg3)
+{
+ return PyObject_CallMethod (obj, name,
+ GDB_PY_INT_ARG GDB_PY_INT_ARG GDB_PY_INT_ARG,
+ arg1, arg2, arg3);
+}
+
/* The 'name' parameter of PyErr_NewException was missing the 'const'
qualifier in Python <= 3.4. Hence, we wrap it in a function to
avoid errors when compiled with -Werror. */
base-commit: 58a628530ee68fe705b443947643037319e7d44e
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod
2024-06-10 18:13 [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod Tom de Vries
@ 2024-06-10 20:09 ` Tom Tromey
2024-06-10 22:23 ` Tom de Vries
0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2024-06-10 20:09 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> Add non-variadic versions of PyObject_CallMethod that hide these details, such
Tom> that we can use the more intuitive:
I think this idea makes sense, but it's inadvisable to overload a
Python-supplied function with gdb-supplied functions of the same name.
I think choosing a different (gdb-specific) name would be better.
Tom> +template<typename T>
Templates shouldn't be needed, ordinary overloading has the same effect.
thanks,
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod
2024-06-10 20:09 ` Tom Tromey
@ 2024-06-10 22:23 ` Tom de Vries
2024-06-11 8:57 ` Tom de Vries
2024-06-11 18:38 ` Tom Tromey
0 siblings, 2 replies; 5+ messages in thread
From: Tom de Vries @ 2024-06-10 22:23 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1975 bytes --]
On 6/10/24 22:09, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
> Tom> Add non-variadic versions of PyObject_CallMethod that hide these details, such
> Tom> that we can use the more intuitive:
>
> I think this idea makes sense, but it's inadvisable to overload a
> Python-supplied function with gdb-supplied functions of the same name.
> I think choosing a different (gdb-specific) name would be better.
>
Done.
I've used gdb_PyObject_CallMethod.
There already was a gdb_PyObject_CallMethod as a compatibility layer,
I've renamed that to gdb_compatibility_PyObject_CallMethod.
> Tom> +template<typename T>
>
> Templates shouldn't be needed, ordinary overloading has the same effect.
That was my initial approach, but I realized that didn't work as I liked.
Consider this test-case:
...
#if 1
int
foo (int i)
{
return 1;
}
#endif
#if 1
int
foo (unsigned int u)
{
return 2;
}
#endif
int
main (void)
{
int i;
unsigned int u;
return foo (u) + foo (i);
}
...
This gets me the expected:
...
$ g++ test.c ; ./a.out; echo $?
3
...
However if I switch off one of the versions of foo, I get 2 or 4 instead.
So, if there are specialized versions for signed and unsigned they are
correctly used, but it's not enforced. And the signed vs unsigned is
relevant because we use different format string chars for those ("i" vs
"I").
Using a templated rewrite:
...
template<typename T>
int
foo (T);
#if 1
template<>
int
foo (int i)
{
return 1;
}
#endif
#if 1
template<>
int
foo (unsigned int u)
{
return 2;
}
#endif
int
main (void)
{
int i;
unsigned int u;
return foo (u) + foo (i);
}
...
I also get:
...
$ g++ test.c ; ./a.out; echo $?
3
...
but if I switch off one of the version of foo, I get a linker error:
...
test.c:(.text+0x1d): undefined reference to `int foo<unsigned
int>(unsigned int)'
...
telling me to add the missing specialization.
Currently retesting this updated version.
Thanks,
- Tom
[-- Attachment #2: 0001-gdb-python-Add-non-variadic-versions-of-PyObject_Cal.patch --]
[-- Type: text/x-patch, Size: 11551 bytes --]
From a0f27d485b1b437f410ca433ec03b6d52502a265 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Mon, 10 Jun 2024 15:34:33 +0200
Subject: [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod
I noticed code like this:
...
gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll",
"i", num_to_scroll, nullptr));
...
The nullptr is superfluous, the format string already indicates that there's
only one argument.
OTOH, passing no args does use a nullptr:
...
gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render",
nullptr));
...
Furthermore, choosing the right format string chars can be tricky.
Add non-variadic versions of PyObject_CallMethod that hide these details, such
that we can use the more intuitive:
...
gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get(), "hscroll",
num_to_scroll));
...
and:
...
gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "render"));
...
Tested on x86_64-linux.
---
gdb/python/py-breakpoint.c | 2 +-
gdb/python/py-disasm.c | 6 ++--
gdb/python/py-finishbreakpoint.c | 4 +--
gdb/python/py-framefilter.c | 17 +++++----
gdb/python/py-tui.c | 20 +++++------
gdb/python/python-internal.h | 61 ++++++++++++++++++++++++++++++--
6 files changed, 81 insertions(+), 29 deletions(-)
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index da74d69d357..ce0096680f5 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1174,7 +1174,7 @@ gdbpy_breakpoint_cond_says_stop (const struct extension_language_defn *extlang,
if (PyObject_HasAttrString (py_bp, stop_func))
{
- gdbpy_ref<> result (PyObject_CallMethod (py_bp, stop_func, NULL));
+ gdbpy_ref<> result (gdb_PyObject_CallMethod (py_bp, stop_func));
stop = 1;
if (result != NULL)
diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 9337514acba..7fd595bf8c4 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -855,9 +855,9 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff,
/* Now call the DisassembleInfo.read_memory method. This might have been
overridden by the user. */
- gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
- "read_memory",
- "I" GDB_PY_LL_ARG, len, offset));
+ gdbpy_ref<> result_obj (gdb_PyObject_CallMethod ((PyObject *) obj,
+ "read_memory",
+ len, offset));
/* Handle any exceptions. */
if (result_obj == nullptr)
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index c74a2473a81..b6f85c202d6 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -344,8 +344,8 @@ bpfinishpy_out_of_scope (struct finish_breakpoint_object *bpfinish_obj)
if (bpfinish_obj->py_bp.bp->enable_state == bp_enabled
&& PyObject_HasAttrString (py_obj, outofscope_func))
{
- gdbpy_ref<> meth_result (PyObject_CallMethod (py_obj, outofscope_func,
- NULL));
+ gdbpy_ref<> meth_result (gdb_PyObject_CallMethod (py_obj,
+ outofscope_func));
if (meth_result == NULL)
gdbpy_print_stack ();
}
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 0cd15977d2f..6ae14c223b5 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -59,7 +59,7 @@ extract_sym (PyObject *obj, gdb::unique_xmalloc_ptr<char> *name,
struct symbol **sym, const struct block **sym_block,
const struct language_defn **language)
{
- gdbpy_ref<> result (PyObject_CallMethod (obj, "symbol", NULL));
+ gdbpy_ref<> result (gdb_PyObject_CallMethod (obj, "symbol"));
if (result == NULL)
return EXT_LANG_BT_ERROR;
@@ -130,7 +130,7 @@ extract_value (PyObject *obj, struct value **value)
{
if (PyObject_HasAttrString (obj, "value"))
{
- gdbpy_ref<> vresult (PyObject_CallMethod (obj, "value", NULL));
+ gdbpy_ref<> vresult (gdb_PyObject_CallMethod (obj, "value"));
if (vresult == NULL)
return EXT_LANG_BT_ERROR;
@@ -264,7 +264,7 @@ get_py_iter_from_func (PyObject *filter, const char *func)
{
if (PyObject_HasAttrString (filter, func))
{
- gdbpy_ref<> result (PyObject_CallMethod (filter, func, NULL));
+ gdbpy_ref<> result (gdb_PyObject_CallMethod (filter, func));
if (result != NULL)
{
@@ -790,8 +790,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
/* Get the underlying frame. This is needed to determine GDB
architecture, and also, in the cases of frame variables/arguments to
read them if they returned filter object requires us to do so. */
- gdbpy_ref<> py_inf_frame (PyObject_CallMethod (filter, "inferior_frame",
- NULL));
+ gdbpy_ref<> py_inf_frame (gdb_PyObject_CallMethod (filter, "inferior_frame"));
if (py_inf_frame == NULL)
return EXT_LANG_BT_ERROR;
@@ -831,7 +830,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
address printing. */
if (PyObject_HasAttrString (filter, "address"))
{
- gdbpy_ref<> paddr (PyObject_CallMethod (filter, "address", NULL));
+ gdbpy_ref<> paddr (gdb_PyObject_CallMethod (filter, "address"));
if (paddr == NULL)
return EXT_LANG_BT_ERROR;
@@ -906,7 +905,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
/* Print frame function name. */
if (PyObject_HasAttrString (filter, "function"))
{
- gdbpy_ref<> py_func (PyObject_CallMethod (filter, "function", NULL));
+ gdbpy_ref<> py_func (gdb_PyObject_CallMethod (filter, "function"));
const char *function = NULL;
if (py_func == NULL)
@@ -970,7 +969,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
if (PyObject_HasAttrString (filter, "filename"))
{
- gdbpy_ref<> py_fn (PyObject_CallMethod (filter, "filename", NULL));
+ gdbpy_ref<> py_fn (gdb_PyObject_CallMethod (filter, "filename"));
if (py_fn == NULL)
return EXT_LANG_BT_ERROR;
@@ -994,7 +993,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
if (PyObject_HasAttrString (filter, "line"))
{
- gdbpy_ref<> py_line (PyObject_CallMethod (filter, "line", NULL));
+ gdbpy_ref<> py_line (gdb_PyObject_CallMethod (filter, "line"));
int line;
if (py_line == NULL)
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 5dcec4bd2b1..57b13148e2e 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -164,8 +164,7 @@ tui_py_window::~tui_py_window ()
if (m_window != nullptr
&& PyObject_HasAttrString (m_window.get (), "close"))
{
- gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "close",
- nullptr));
+ gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "close"));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -198,8 +197,7 @@ tui_py_window::rerender ()
if (PyObject_HasAttrString (m_window.get (), "render"))
{
- gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render",
- nullptr));
+ gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "render"));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -212,8 +210,8 @@ tui_py_window::do_scroll_horizontal (int num_to_scroll)
if (PyObject_HasAttrString (m_window.get (), "hscroll"))
{
- gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll",
- "i", num_to_scroll, nullptr));
+ gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "hscroll",
+ num_to_scroll));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -226,8 +224,8 @@ tui_py_window::do_scroll_vertical (int num_to_scroll)
if (PyObject_HasAttrString (m_window.get (), "vscroll"))
{
- gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "vscroll",
- "i", num_to_scroll, nullptr));
+ gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "vscroll",
+ num_to_scroll));
if (result == nullptr)
gdbpy_print_stack ();
}
@@ -248,9 +246,9 @@ tui_py_window::click (int mouse_x, int mouse_y, int mouse_button)
if (PyObject_HasAttrString (m_window.get (), "click"))
{
- gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "click",
- "iii", mouse_x, mouse_y,
- mouse_button));
+ gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "click",
+ mouse_x, mouse_y,
+ mouse_button));
if (result == nullptr)
gdbpy_print_stack ();
}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index d07f239ca65..6b8ec18c6a0 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -100,6 +100,12 @@
#define PyEval_ReleaseLock()
#endif
+/* Format string chars as defined here [1], in a readable and intuitive form.
+ [1] https://docs.python.org/3/c-api/arg.html#building-values. */
+
+#define GDB_PY_INT_ARG "i"
+#define GDB_PY_UNSIGNED_INT_ARG "I"
+
/* Python supplies HAVE_LONG_LONG and some `long long' support when it
is available. These defines let us handle the differences more
cleanly.
@@ -154,8 +160,10 @@ typedef long Py_hash_t;
template<typename... Args>
static inline PyObject *
-gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format,
- Args... args) /* ARI: editCase function */
+gdb_compatibility_PyObject_CallMethod (PyObject *o, const char *method,
+ const char *format,
+ Args... args) /* ARI: editCase
+ function. */
{
return PyObject_CallMethod (o,
const_cast<char *> (method),
@@ -164,7 +172,54 @@ gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format,
}
#undef PyObject_CallMethod
-#define PyObject_CallMethod gdb_PyObject_CallMethod
+#define PyObject_CallMethod gdb_compatibility_PyObject_CallMethod
+
+/* Non-variadic versions of PyObject_CallMethod. */
+
+static inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name)
+{
+ return PyObject_CallMethod (obj, name, nullptr);
+}
+
+template<typename T>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, int arg1)
+{
+ return PyObject_CallMethod (obj, name, GDB_PY_INT_ARG, arg1);
+}
+
+template<typename T, typename T2>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T, T2);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, unsigned int arg1,
+ gdb_py_longest arg2)
+{
+ return PyObject_CallMethod (obj, name,
+ GDB_PY_UNSIGNED_INT_ARG GDB_PY_LL_ARG,
+ arg1, arg2);
+}
+
+template<typename T, typename T2, typename T3>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T, T2, T3);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, int arg1, int arg2,
+ int arg3)
+{
+ return PyObject_CallMethod (obj, name,
+ GDB_PY_INT_ARG GDB_PY_INT_ARG GDB_PY_INT_ARG,
+ arg1, arg2, arg3);
+}
/* The 'name' parameter of PyErr_NewException was missing the 'const'
qualifier in Python <= 3.4. Hence, we wrap it in a function to
base-commit: 58a628530ee68fe705b443947643037319e7d44e
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod
2024-06-10 22:23 ` Tom de Vries
@ 2024-06-11 8:57 ` Tom de Vries
2024-06-11 18:38 ` Tom Tromey
1 sibling, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2024-06-11 8:57 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 6/11/24 00:23, Tom de Vries wrote:
> On 6/10/24 22:09, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> Add non-variadic versions of PyObject_CallMethod that hide these
>> details, such
>> Tom> that we can use the more intuitive:
>>
>> I think this idea makes sense, but it's inadvisable to overload a
>> Python-supplied function with gdb-supplied functions of the same name.
>> I think choosing a different (gdb-specific) name would be better.
>>
>
> Done.
>
> I've used gdb_PyObject_CallMethod.
>
> There already was a gdb_PyObject_CallMethod as a compatibility layer,
> I've renamed that to gdb_compatibility_PyObject_CallMethod.
>
>> Tom> +template<typename T>
>>
>> Templates shouldn't be needed, ordinary overloading has the same effect.
>
> That was my initial approach, but I realized that didn't work as I liked.
>
> Consider this test-case:
> ...
> #if 1
> int
> foo (int i)
> {
> return 1;
> }
> #endif
>
> #if 1
> int
> foo (unsigned int u)
> {
> return 2;
> }
> #endif
>
> int
> main (void)
> {
> int i;
> unsigned int u;
>
> return foo (u) + foo (i);
> }
> ...
>
> This gets me the expected:
> ...
> $ g++ test.c ; ./a.out; echo $?
> 3
> ...
>
> However if I switch off one of the versions of foo, I get 2 or 4 instead.
>
> So, if there are specialized versions for signed and unsigned they are
> correctly used, but it's not enforced. And the signed vs unsigned is
> relevant because we use different format string chars for those ("i" vs
> "I").
>
> Using a templated rewrite:
> ...
> template<typename T>
> int
> foo (T);
>
> #if 1
> template<>
> int
> foo (int i)
> {
> return 1;
> }
> #endif
>
> #if 1
> template<>
> int
> foo (unsigned int u)
> {
> return 2;
> }
> #endif
>
> int
> main (void)
> {
> int i;
> unsigned int u;
>
> return foo (u) + foo (i);
> }
> ...
> I also get:
> ...
> $ g++ test.c ; ./a.out; echo $?
> 3
> ...
> but if I switch off one of the version of foo, I get a linker error:
> ...
> test.c:(.text+0x1d): undefined reference to `int foo<unsigned
> int>(unsigned int)'
> ...
> telling me to add the missing specialization.
>
> Currently retesting this updated version.
>
And submitted as v2 (
https://sourceware.org/pipermail/gdb-patches/2024-June/209847.html )
with one formatting nit fixed.
Thanks,
- Tom
> Thanks,
> - Tom
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod
2024-06-10 22:23 ` Tom de Vries
2024-06-11 8:57 ` Tom de Vries
@ 2024-06-11 18:38 ` Tom Tromey
1 sibling, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2024-06-11 18:38 UTC (permalink / raw)
To: Tom de Vries; +Cc: Tom Tromey, gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> There already was a gdb_PyObject_CallMethod as a compatibility layer,
Tom> I've renamed that to gdb_compatibility_PyObject_CallMethod.
It seems to me that this could just be removed and we could simply
always use the wrappers in gdb.
Tom> -#define PyObject_CallMethod gdb_PyObject_CallMethod
Tom> +#define PyObject_CallMethod gdb_compatibility_PyObject_CallMethod
I forgot about and/or never saw this define, but if gdb always used the
wrappers I guess this could be redefined to give some error; or perhaps
poisoned.
Tom> +static inline PyObject *
Tom> +gdb_PyObject_CallMethod (PyObject *obj, const char *name)
Tom> +{
Tom> + return PyObject_CallMethod (obj, name, nullptr);
All of these specializations need the const_cast in order to work with
Python versions earlier than 3.4. Assuming we support those.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-11 18:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-10 18:13 [PATCH] [gdb/python] Add non-variadic versions of PyObject_CallMethod Tom de Vries
2024-06-10 20:09 ` Tom Tromey
2024-06-10 22:23 ` Tom de Vries
2024-06-11 8:57 ` Tom de Vries
2024-06-11 18:38 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).