public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 13/18] -Wwrite-strings: Wrap PyGetSetDef for construction with string literals
Date: Tue, 04 Apr 2017 17:32:00 -0000	[thread overview]
Message-ID: <1491326751-16180-14-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1491326751-16180-1-git-send-email-palves@redhat.com>

Unfortunately, PyGetSetDef's 'name' and 'doc' members are 'char *'
instead of 'const char *', meaning that in order to list-initialize
PyGetSetDef arrays using string literals requires writing explicit
'char *' casts.  For example:

    static PyGetSetDef value_object_getset[] = {
   -  { "address", valpy_get_address, NULL, "The address of the value.",
   +  { (char *) "address", valpy_get_address, NULL,
   +    (char *) "The address of the value.",
	NULL },
   -  { "is_optimized_out", valpy_get_is_optimized_out, NULL,
   -    "Boolean telling whether the value is optimized "
   +  { (char *) "is_optimized_out", valpy_get_is_optimized_out, NULL,
   +    (char *) "Boolean telling whether the value is optimized "
	"out (i.e., not available).",
	NULL },
   -  { "type", valpy_get_type, NULL, "Type of the value.", NULL },
   -  { "dynamic_type", valpy_get_dynamic_type, NULL,
   -    "Dynamic type of the value.", NULL },
   -  { "is_lazy", valpy_get_is_lazy, NULL,
   -    "Boolean telling whether the value is lazy (not fetched yet\n\
   +  { (char *) "type", valpy_get_type, NULL,
   +    (char *) "Type of the value.", NULL },
   +  { (char *) "dynamic_type", valpy_get_dynamic_type, NULL,
   +    (char *) "Dynamic type of the value.", NULL },
   +  { (char *) "is_lazy", valpy_get_is_lazy, NULL,
   +    (char *) "Boolean telling whether the value is lazy (not fetched yet\n\
    from the inferior).  A lazy value is fetched when needed, or when\n\
    the \"fetch_lazy()\" method is called.", NULL },
      {NULL}  /* Sentinel */

We have 20 such arrays, and I first wrote a patch that fixed all of
them like that...  It's not pretty, and I can post it if people want
to see it.

One way to make these a bit less ugly would be add a new macro that
hides the casts, like:

  #define GDBPY_GSDEF(NAME, GET, SET, DOC, CLOSURE) \
     { (char *) NAME, GET, SET, (char *) DOC, CLOSURE }

and then use it like:

    static PyGetSetDef value_object_getset[] = {
       GDBPY_GSDEF ("address", valpy_get_address, NULL,
       		    "The address of the value.", NULL),
       GDBPY_GSDEF ("is_optimized_out", valpy_get_is_optimized_out, NULL,
       		    "Boolean telling whether the value is optimized ", NULL),
      {NULL}  /* Sentinel */
    };

But since we have C++11, which gives us constexpr and list
initialization, I thought of a way that requires no changes where the
arrays are initialized:

We add a new type that extends PyGetSetDef (called gdb_PyGetSetDef),
and add constexpr constructors that accept const 'name' and 'doc', and
then list/aggregate initialization simply "calls" these matching
constructors instead.

I put "calls" in quotes, because given "constexpr", it's all done at
compile time, and there's no overhead either in binary size or at run
time.  In fact, we get identical binaries, before/after this change.

I'm a bit undecided whether to change the places that create
PyGetSetDef arrays to explicitly name gdb_PyGetSetDef as type, like:

  -    static PyGetSetDef value_object_getset[] = {
  +    static gdb_PyGetSetDef value_object_getset[] = {

or go the

  #define PyGetSetDef gdb_PyGetSetDef

way as we do for the other Python API fixes.  This commit takes the
latter approach, but I'll change it if people prefer the other way.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* python/python-internal.h (gdb_PyGetSetDef): New type.
	(PyGetSetDef): New define.
---
 gdb/python/python-internal.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 55efd75..8fc89cd 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -286,6 +286,38 @@ gdb_PySys_SetPath (const GDB_PYSYS_SETPATH_CHAR *path)
 
 #define PySys_SetPath gdb_PySys_SetPath
 
+/* Wrap PyGetSetDef to allow convenient construction with string
+   literals.  Unfortunately, PyGetSetDef's 'name' and 'doc' members
+   are 'char *' instead of 'const char *', meaning that in order to
+   list-initialize PyGetSetDef arrays with string literals (and
+   without the wrapping below) would require writing explicit 'char *'
+   casts.  Instead, we extend PyGetSetDef and add onstexpr
+   constructors that accept const 'name' and 'doc', hiding the ugly
+   casts here in a single place.  */
+
+struct gdb_PyGetSetDef : PyGetSetDef
+{
+  constexpr gdb_PyGetSetDef (const char *name_, getter get_, setter set_,
+			     const char *doc_, void *closure_)
+    : PyGetSetDef {const_cast<char *> (name_), get_, set_,
+		   const_cast<char *> (doc_), closure_}
+  {}
+
+  /* Alternative constructor that allows omitting the closure in list
+     initialization.  */
+  constexpr gdb_PyGetSetDef (const char *name_, getter get_, setter set_,
+			     const char *doc_)
+    : gdb_PyGetSetDef {name_, get_, set_, doc_, NULL}
+  {}
+
+  /* Constructor for the sentinel entries.  */
+  constexpr gdb_PyGetSetDef (std::nullptr_t)
+    : gdb_PyGetSetDef { NULL, NULL, NULL, NULL, NULL }
+  {}
+};
+
+#define PyGetSetDef gdb_PyGetSetDef
+
 /* In order to be able to parse symtab_and_line_to_sal_object function
    a real symtab_and_line structure is needed.  */
 #include "symtab.h"
-- 
2.5.5

  parent reply	other threads:[~2017-04-04 17:32 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 17:25 [PATCH 00/18] gdb: Enable -Wwrite-strings (aka remove -Wno-write-strings) Pedro Alves
2017-04-04 17:26 ` [PATCH 16/18] -Wwrite-strings: Some constification in gdb/breakpoint.c Pedro Alves
2017-04-04 17:26 ` [PATCH 15/18] -Wwrite-strings: execute_command calls with string literals Pedro Alves
2017-04-05  7:13   ` Metzger, Markus T
2017-04-05 13:10     ` Pedro Alves
2017-04-04 17:26 ` [PATCH 09/18] -Wwrite-strings: MI -info-os Pedro Alves
2017-04-04 17:26 ` [PATCH 07/18] -Wwrite-strings: Constify work break character arrays Pedro Alves
2017-04-05  8:46   ` Philipp Rudo
2017-04-05 13:17     ` Pedro Alves
2017-04-04 17:26 ` [PATCH 04/18] -Wwrite-strings: Constify shell_escape and plug make_command leak Pedro Alves
2017-04-04 17:26 ` [PATCH 03/18] -Wwrite-strings: Don't initialize string command variables to empty string Pedro Alves
2017-04-04 17:26 ` [PATCH 06/18] -Wwrite-strings: Constify target_pid_to_str and target_thread_extra_thread_info Pedro Alves
2017-04-04 18:44   ` John Baldwin
2017-04-04 17:26 ` [PATCH 05/18] -Wwrite-strings: Constify warning_pre_print Pedro Alves
2017-04-04 17:26 ` [PATCH 02/18] -Wwrite-strings: Constify macroexp.c:init_shared_buffer Pedro Alves
2017-04-04 17:26 ` [PATCH 08/18] -Wwrite-strings: Constify mi_cmd_argv_ftype's 'command' parameter Pedro Alves
2017-04-04 17:26 ` [PATCH 01/18] -Wwrite-strings: Constify struct disassemble_info's disassembler_options field Pedro Alves
2017-04-05  7:22   ` Nick Clifton
2017-04-04 17:26 ` [PATCH 14/18] -Wwrite-strings: Add a PyArg_ParseTupleAndKeywords "const char *" overload Pedro Alves
2017-04-04 18:37   ` Sergio Durigan Junior
2017-04-05 12:58     ` Pedro Alves
2017-04-05 15:49       ` Sergio Durigan Junior
2017-04-04 17:31 ` [PATCH 10/18] -Wwrite-strings: gdbserver's 'port' parsing Pedro Alves
2017-04-04 17:32 ` [PATCH 11/18] -Wwrite-strings: gdbserver/win32-low.c and TARGET_WAITKIND_EXECD Pedro Alves
2017-04-04 17:32 ` Pedro Alves [this message]
2017-04-04 18:40   ` [PATCH 13/18] -Wwrite-strings: Wrap PyGetSetDef for construction with string literals Sergio Durigan Junior
2017-04-05 12:35     ` Pedro Alves
2017-04-05 15:48       ` Sergio Durigan Junior
2017-04-05  8:49   ` Philipp Rudo
2017-04-05 13:03     ` Pedro Alves
2017-04-04 17:32 ` [PATCH 18/18] -Wwrite-strings: Remove -Wno-write-strings Pedro Alves
2019-02-14 16:17   ` Thomas Schwinge
2017-04-04 17:33 ` [PATCH 12/18] -Wwrite-strings: More fix-old-Python-API wrappers Pedro Alves
2017-04-04 17:36 ` [PATCH 17/18] -Wwrite-strings: The Rest Pedro Alves
2017-04-04 18:44   ` John Baldwin
2017-04-05 12:59     ` Pedro Alves
2017-04-04 18:42 ` [PATCH 00/18] gdb: Enable -Wwrite-strings (aka remove -Wno-write-strings) Sergio Durigan Junior
2017-04-04 19:37 ` Simon Marchi
2017-04-05 18:05 ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1491326751-16180-14-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).