From: Pedro Alves <palves@redhat.com>
Date: Wed, 5 Apr 2017 18:21:36 +0000 (+0100)
Subject: -Wwrite-strings: Wrap PyGetSetDef for construction with string literals
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=0d1f4ceb3904c4c82231adf98f0e84f37bc8d4ea;p=binutils-gdb.git

-Wwrite-strings: Wrap PyGetSetDef for 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 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...

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.

Unlike the fixes that fix some old Python API to match the API of more
recent Python, this switches to using explicit "gdb_PyGetSetDef"
everywhere, just to be clear that we are using our own version of it.

gdb/ChangeLog:
2017-04-05  Pedro Alves  <palves@redhat.com>

	* python/python-internal.h (gdb_PyGetSetDef): New type.
	* python/py-block.c (block_object_getset)
	(breakpoint_object_getset): Now a gdb_PyGetSetDef array.
	* python/py-event.c (event_object_getset)
	(finish_breakpoint_object_getset): Likewise.
	* python/py-inferior.c (inferior_object_getset): Likewise.
	* python/py-infthread.c (thread_object_getset): Likewise.
	* python/py-lazy-string.c (lazy_string_object_getset): Likewise.
	* python/py-linetable.c (linetable_entry_object_getset): Likewise.
	* python/py-objfile.c (objfile_getset): Likewise.
	* python/py-progspace.c (pspace_getset): Likewise.
	* python/py-record-btrace.c (btpy_insn_getset, btpy_call_getset):
	Likewise.
	* python/py-record.c (recpy_record_getset): Likewise.
	* python/py-symbol.c (symbol_object_getset): Likewise.
	* python/py-symtab.c (symtab_object_getset, sal_object_getset):
	Likewise.
	* python/py-type.c (type_object_getset, field_object_getset):
	Likewise.
	* python/py-value.c (value_object_getset): Likewise.
---

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9430d3a273a..28557c27005 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,26 @@
+2017-04-05  Pedro Alves  <palves@redhat.com>
+
+	* python/python-internal.h (gdb_PyGetSetDef): New type.
+	* python/py-block.c (block_object_getset)
+	(breakpoint_object_getset): Now a gdb_PyGetSetDef array.
+	* python/py-event.c (event_object_getset)
+	(finish_breakpoint_object_getset): Likewise.
+	* python/py-inferior.c (inferior_object_getset): Likewise.
+	* python/py-infthread.c (thread_object_getset): Likewise.
+	* python/py-lazy-string.c (lazy_string_object_getset): Likewise.
+	* python/py-linetable.c (linetable_entry_object_getset): Likewise.
+	* python/py-objfile.c (objfile_getset): Likewise.
+	* python/py-progspace.c (pspace_getset): Likewise.
+	* python/py-record-btrace.c (btpy_insn_getset, btpy_call_getset):
+	Likewise.
+	* python/py-record.c (recpy_record_getset): Likewise.
+	* python/py-symbol.c (symbol_object_getset): Likewise.
+	* python/py-symtab.c (symtab_object_getset, sal_object_getset):
+	Likewise.
+	* python/py-type.c (type_object_getset, field_object_getset):
+	Likewise.
+	* python/py-value.c (value_object_getset): Likewise.
+
 2017-04-05  Pedro Alves  <palves@redhat.com>
 
 	* python/python-internal.h (gdb_PyObject_CallMethod)
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index f477d4a63e1..840c842e85a 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -461,7 +461,7 @@ Return true if this block is valid, false if not." },
   {NULL}  /* Sentinel */
 };
 
-static PyGetSetDef block_object_getset[] = {
+static gdb_PyGetSetDef block_object_getset[] = {
   { "start", blpy_get_start, NULL, "Start address of the block.", NULL },
   { "end", blpy_get_end, NULL, "End address of the block.", NULL },
   { "function", blpy_get_function, NULL,
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 724a7ed41c9..34f46fb7a42 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1048,7 +1048,7 @@ local_setattro (PyObject *self, PyObject *name, PyObject *v)
   return PyObject_GenericSetAttr ((PyObject *)self, name, v);
 }
 
-static PyGetSetDef breakpoint_object_getset[] = {
+static gdb_PyGetSetDef breakpoint_object_getset[] = {
   { "enabled", bppy_get_enabled, bppy_set_enabled,
     "Boolean telling whether the breakpoint is enabled.", NULL },
   { "silent", bppy_get_silent, bppy_set_silent,
diff --git a/gdb/python/py-event.c b/gdb/python/py-event.c
index 127dcc74260..dc1d73e78b4 100644
--- a/gdb/python/py-event.c
+++ b/gdb/python/py-event.c
@@ -114,7 +114,7 @@ evpy_emit_event (PyObject *event,
   return 0;
 }
 
-static PyGetSetDef event_object_getset[] =
+static gdb_PyGetSetDef event_object_getset[] =
 {
   { "__dict__", gdb_py_generic_dict, NULL,
     "The __dict__ for this event.", &event_object_type },
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 106fe34e85a..76189b8a1de 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -426,7 +426,7 @@ gdbpy_initialize_finishbreakpoints (void)
   return 0;
 }
 
-static PyGetSetDef finish_breakpoint_object_getset[] = {
+static gdb_PyGetSetDef finish_breakpoint_object_getset[] = {
   { "return_value", bpfinishpy_get_returnvalue, NULL,
   "gdb.Value object representing the return value, if any. \
 None otherwise.", NULL },
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 46a0aad593c..77fc543dcab 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -827,7 +827,7 @@ gdbpy_initialize_inferior (void)
 				 &membuf_object_type);
 }
 
-static PyGetSetDef inferior_object_getset[] =
+static gdb_PyGetSetDef inferior_object_getset[] =
 {
   { "num", infpy_get_num, NULL, "ID of inferior, as assigned by GDB.", NULL },
   { "pid", infpy_get_pid, NULL, "PID of inferior, as assigned by the OS.",
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 5482bf9ea19..626c15cedb5 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -304,7 +304,7 @@ gdbpy_initialize_thread (void)
 				 (PyObject *) &thread_object_type);
 }
 
-static PyGetSetDef thread_object_getset[] =
+static gdb_PyGetSetDef thread_object_getset[] =
 {
   { "name", thpy_get_name, thpy_set_name,
     "The name of the thread, as set by the user or the OS.", NULL },
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index ab3f4117439..19990330601 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -300,7 +300,7 @@ static PyMethodDef lazy_string_object_methods[] = {
 };
 
 
-static PyGetSetDef lazy_string_object_getset[] = {
+static gdb_PyGetSetDef lazy_string_object_getset[] = {
   { "address", stpy_get_address, NULL, "Address of the string.", NULL },
   { "encoding", stpy_get_encoding, NULL, "Encoding of the string.", NULL },
   { "length", stpy_get_length, NULL, "Length of the string.", NULL },
diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index a5e57b0d365..8d17aabbf9d 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -550,7 +550,7 @@ PyTypeObject ltpy_iterator_object_type = {
 };
 
 
-static PyGetSetDef linetable_entry_object_getset[] = {
+static gdb_PyGetSetDef linetable_entry_object_getset[] = {
   { "line", ltpy_entry_get_line, NULL,
     "The line number in the source file.", NULL },
   { "pc", ltpy_entry_get_pc, NULL,
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 105d88a27c7..6a47c170340 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -670,7 +670,7 @@ Add FILE_NAME to the list of files containing debug info for the objfile." },
   { NULL }
 };
 
-static PyGetSetDef objfile_getset[] =
+static gdb_PyGetSetDef objfile_getset[] =
 {
   { "__dict__", gdb_py_generic_dict, NULL,
     "The __dict__ for this objfile.", &objfile_object_type },
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 1e06a75d2f7..edabba4a002 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -378,7 +378,7 @@ gdbpy_initialize_pspace (void)
 
 
 
-static PyGetSetDef pspace_getset[] =
+static gdb_PyGetSetDef pspace_getset[] =
 {
   { "__dict__", gdb_py_generic_dict, NULL,
     "The __dict__ for this progspace.", &pspace_object_type },
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index c8163326d01..6d08121376e 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -903,7 +903,7 @@ recpy_bt_goto (PyObject *self, PyObject *args)
 
 /* BtraceInstruction members.  */
 
-struct PyGetSetDef btpy_insn_getset[] =
+struct gdb_PyGetSetDef btpy_insn_getset[] =
 {
   { "number", btpy_number, NULL, "instruction number", NULL},
   { "error", btpy_insn_error, NULL, "error number for gaps", NULL},
@@ -920,7 +920,7 @@ executed speculatively", NULL},
 
 /* BtraceFunctionCall members.  */
 
-static PyGetSetDef btpy_call_getset[] =
+static gdb_PyGetSetDef btpy_call_getset[] =
 {
   { "number", btpy_number, NULL, "function call number", NULL},
   { "level", btpy_call_level, NULL, "call stack level", NULL},
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index 72922a41db2..60c0a7ce92b 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -175,7 +175,7 @@ Rewind to given location."},
 
 /* Record member list.  */
 
-static PyGetSetDef recpy_record_getset[] = {
+static gdb_PyGetSetDef recpy_record_getset[] = {
   { "ptid", recpy_ptid, NULL, "Current thread.", NULL },
   { "method", recpy_method, NULL, "Current recording method.", NULL },
   { "format", recpy_format, NULL, "Current recording format.", NULL },
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index b71cfb404fe..05b002fe725 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -560,7 +560,7 @@ gdbpy_initialize_symbols (void)
 
 
 
-static PyGetSetDef symbol_object_getset[] = {
+static gdb_PyGetSetDef symbol_object_getset[] = {
   { "type", sympy_get_type, NULL,
     "Type of the symbol.", NULL },
   { "symtab", sympy_get_symtab, NULL,
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 09cab221820..53b160e5d2c 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -544,7 +544,7 @@ gdbpy_initialize_symtabs (void)
 
 
 
-static PyGetSetDef symtab_object_getset[] = {
+static gdb_PyGetSetDef symtab_object_getset[] = {
   { "filename", stpy_get_filename, NULL,
     "The symbol table's source filename.", NULL },
   { "objfile", stpy_get_objfile, NULL, "The symtab's objfile.",
@@ -606,7 +606,7 @@ PyTypeObject symtab_object_type = {
   symtab_object_getset		  /*tp_getset */
 };
 
-static PyGetSetDef sal_object_getset[] = {
+static gdb_PyGetSetDef sal_object_getset[] = {
   { "symtab", salpy_get_symtab, NULL, "Symtab object.", NULL },
   { "pc", salpy_get_pc, NULL, "Return the symtab_and_line's pc.", NULL },
   { "last", salpy_get_last, NULL,
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index f0710061f74..12b631086d9 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1413,7 +1413,7 @@ gdbpy_initialize_types (void)
 
 
 
-static PyGetSetDef type_object_getset[] =
+static gdb_PyGetSetDef type_object_getset[] =
 {
   { "code", typy_get_code, NULL,
     "The code for this type.", NULL },
@@ -1587,7 +1587,7 @@ PyTypeObject type_object_type =
   0,				  /* tp_new */
 };
 
-static PyGetSetDef field_object_getset[] =
+static gdb_PyGetSetDef field_object_getset[] =
 {
   { "__dict__", gdb_py_generic_dict, NULL,
     "The __dict__ for this field.", &field_object_type },
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index bb42e8b88e6..9c0470f8127 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1767,7 +1767,7 @@ gdbpy_initialize_values (void)
 
 
 
-static PyGetSetDef value_object_getset[] = {
+static gdb_PyGetSetDef value_object_getset[] = {
   { "address", valpy_get_address, NULL, "The address of the value.",
     NULL },
   { "is_optimized_out", valpy_get_is_optimized_out, NULL,
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 55efd75a8b6..027faa59fa4 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -286,6 +286,36 @@ 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 constexpr
+   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}
+  {}
+};
+
 /* 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"