From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9618 invoked by alias); 5 Apr 2017 12:35:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 6389 invoked by uid 89); 5 Apr 2017 12:35:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy= X-HELO: mail-wr0-f177.google.com Received: from mail-wr0-f177.google.com (HELO mail-wr0-f177.google.com) (209.85.128.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Apr 2017 12:35:22 +0000 Received: by mail-wr0-f177.google.com with SMTP id w43so12111279wrb.0 for ; Wed, 05 Apr 2017 05:35:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=yQ6kzjKChiWj95nNWQ7h+DGshjx2T4PjU8OjDf+jEPA=; b=NeUpilpJ66eOtnLuYt7eyzJA/xZ0z6h9x6TNX64/AJQxsgc/NiLaaRGPRe35zZCdf/ nSPFaod5jlcg0p4FsgxuqI+V2UQzBGwcu7OCTPCuH185iSTvUkpqNBkoYKWwOCDAgtJX TeDzWRLzDadpKABR3Hkv2Rra/QcasPtFSEYlNwP89xbszkxiXmmvEjHZtcrFhyfN8rYa sGftVdoc4g1M0MUvAo9zf1BtScfpKt8SqgzQxH6t2lOG0sCMunxkQsEo+oeG5/GvFOlt Rsi66KFncSNxcTnS74fRRkuKJlW2yDAT91nDLM5dTLJfYXp9Z0r8JqcutAlb9jCa2o0u VRxg== X-Gm-Message-State: AFeK/H3B19X9W50o+haiFUrPccZzQ0re3TwzyckJxrfKhQH5K/BqMCD8KSrk68Y1k9oQfwQr X-Received: by 10.28.199.132 with SMTP id x126mr18301789wmf.37.1491395721231; Wed, 05 Apr 2017 05:35:21 -0700 (PDT) Received: from [192.168.0.100] ([37.189.166.198]) by smtp.gmail.com with ESMTPSA id y11sm2368503wrd.22.2017.04.05.05.35.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Apr 2017 05:35:20 -0700 (PDT) Subject: Re: [PATCH 13/18] -Wwrite-strings: Wrap PyGetSetDef for construction with string literals To: Sergio Durigan Junior References: <1491326751-16180-1-git-send-email-palves@redhat.com> <1491326751-16180-14-git-send-email-palves@redhat.com> <877f30dnk0.fsf@redhat.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Wed, 05 Apr 2017 12:35:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <877f30dnk0.fsf@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-04/txt/msg00095.txt.bz2 On 04/04/2017 07:39 PM, Sergio Durigan Junior wrote: > I liked the approach. As with the other Python patch, I'm more inclined > to use an explicit "gdb_PyGetSetDef" everywhere, just to be clear that > we are using our own version of it. This can save time when debugging. OK, here's what that looks like. Commit log updated to reflect the change too. WDYT? >From 566fd938f2919985db6a955d769ba619534964e8 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 5 Apr 2017 13:22:25 +0100 Subject: [PATCH] -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: yyyy-mm-dd Pedro Alves * 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. --- gdb/python/py-block.c | 2 +- gdb/python/py-breakpoint.c | 2 +- gdb/python/py-event.c | 2 +- gdb/python/py-finishbreakpoint.c | 2 +- gdb/python/py-inferior.c | 2 +- gdb/python/py-infthread.c | 2 +- gdb/python/py-lazy-string.c | 2 +- gdb/python/py-linetable.c | 2 +- gdb/python/py-objfile.c | 2 +- gdb/python/py-progspace.c | 2 +- gdb/python/py-record-btrace.c | 4 ++-- gdb/python/py-record.c | 2 +- gdb/python/py-symbol.c | 2 +- gdb/python/py-symtab.c | 4 ++-- gdb/python/py-type.c | 4 ++-- gdb/python/py-value.c | 2 +- gdb/python/python-internal.h | 30 ++++++++++++++++++++++++++++++ 17 files changed, 49 insertions(+), 19 deletions(-) diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c index f477d4a..840c842 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 724a7ed..34f46fb 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 127dcc7..dc1d73e 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 106fe34..76189b8 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 46a0aad..77fc543 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 5482bf9..626c15c 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 ab3f411..1999033 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 a5e57b0..8d17aab 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 105d88a..6a47c17 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 1e06a75..edabba4 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 c816332..6d08121 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 72922a4..60c0a7c 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 b71cfb4..05b002f 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 09cab22..53b160e 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 f071006..12b6310 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 bb42e8b..9c0470f 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 55efd75..d4ed170 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 (name_), get_, set_, + const_cast (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" -- 2.5.5