From d4b0bb186e204f77ed70bc719d16c6ca302094fd Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Tue, 10 Jan 2017 23:34:22 -0700 Subject: [PATCH] Remove some ui_out-related cleanups from Python This patch introduces a bit of infrastructure -- namely, a minimal std::optional analogue called gdb::optional, and an RAII template class that works like make_cleanup_ui_out_tuple_begin_end or make_cleanup_ui_out_list_begin_end -- and then uses these in the Python code. This removes a number of cleanups and generally simplifies this code. std::optional is only available in C++17. Normally I would have had this code check __cplusplus, but my gcc apparently isn't new enough to find , even with -std=c++1z; so, because I could not test it, the patch does not do this. gdb/ChangeLog 2017-02-10 Tom Tromey * ui-out.h (ui_out_emit_type): New class. (ui_out_emit_tuple, ui_out_emit_list): New typedefs. * python/py-framefilter.c (py_print_single_arg): Use gdb::optional and ui_out_emit_tuple. (enumerate_locals): Likewise. (py_mi_print_variables, py_print_locals, py_print_args): Use ui_out_emit_list. (py_print_frame): Use gdb::optional, ui_out_emit_tuple, ui_out_emit_list. * common/gdb_optional.h: New file. --- gdb/ChangeLog | 13 +++ gdb/common/gdb_optional.h | 87 +++++++++++++++ gdb/python/py-framefilter.c | 209 +++++++++--------------------------- gdb/ui-out.h | 33 ++++++ 4 files changed, 186 insertions(+), 156 deletions(-) create mode 100644 gdb/common/gdb_optional.h diff --git a/gdb/ChangeLog b/gdb/ChangeLog index e3709ead849..1fe257dcb07 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2017-02-10 Tom Tromey + + * ui-out.h (ui_out_emit_type): New class. + (ui_out_emit_tuple, ui_out_emit_list): New typedefs. + * python/py-framefilter.c (py_print_single_arg): Use gdb::optional + and ui_out_emit_tuple. + (enumerate_locals): Likewise. + (py_mi_print_variables, py_print_locals, py_print_args): Use + ui_out_emit_list. + (py_print_frame): Use gdb::optional, ui_out_emit_tuple, + ui_out_emit_list. + * common/gdb_optional.h: New file. + 2017-02-10 Martin Galvan * MAINTAINERS (Write After Approval): Update my e-mail address. diff --git a/gdb/common/gdb_optional.h b/gdb/common/gdb_optional.h new file mode 100644 index 00000000000..d991da123cf --- /dev/null +++ b/gdb/common/gdb_optional.h @@ -0,0 +1,87 @@ +/* An optional object. + + Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef GDB_OPTIONAL_H +#define GDB_OPTIONAL_H + +namespace gdb +{ + +/* This class attempts to be a compatible subset of std::optional, + which is slated to be available in C++17. This class optionally + holds an object of some type -- by default it is constructed not + holding an object, but later the object can be "emplaced". This is + similar to using std::unique_ptr, but in-object allocation is + guaranteed. */ +template +class optional +{ +public: + + optional () + : m_instantiated (false) + { + } + + ~optional () + { + if (m_instantiated) + destroy (); + } + + /* These aren't deleted in std::optional, but it was simpler to + delete them here, because currently the users of this class don't + need them, and making them depend on the definition of T is + somewhat complicated. */ + optional (const optional &other) = delete; + optional &operator= (const optional &other) = delete; + + template + void emplace (Args &&... args) + { + if (m_instantiated) + destroy (); + new (&m_item) T (std::forward(args)...); + m_instantiated = true; + } + +private: + + /* Destroy the object. */ + void destroy () + { + gdb_assert (m_instantiated); + m_instantiated = false; + m_item.~T (); + } + + /* The object. */ + union + { + struct { } m_dummy; + T m_item; + }; + + /* True if the object was ever emplaced. */ + bool m_instantiated; +}; + +} + +#endif /* GDB_OPTIONAL_H */ diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c index 9dc01bad14a..a4a22a090de 100644 --- a/gdb/python/py-framefilter.c +++ b/gdb/python/py-framefilter.c @@ -31,6 +31,7 @@ #include "mi/mi-cmds.h" #include "python-internal.h" #include "py-ref.h" +#include "common/gdb_optional.h" enum mi_print_types { @@ -365,7 +366,7 @@ py_print_single_arg (struct ui_out *out, TRY { - struct cleanup *cleanups = make_cleanup (null_cleanup, NULL); + gdb::optional maybe_tuple; /* MI has varying rules for tuples, but generally if there is only one element in each item in the list, do not start a tuple. The @@ -376,7 +377,7 @@ py_print_single_arg (struct ui_out *out, if (out->is_mi_like_p ()) { if (print_args_field || args_type != NO_VALUES) - make_cleanup_ui_out_tuple_begin_end (out, NULL); + maybe_tuple.emplace (out, nullptr); } annotate_arg_begin (); @@ -421,10 +422,7 @@ py_print_single_arg (struct ui_out *out, if (args_type == MI_PRINT_SIMPLE_VALUES && val != NULL) { if (py_print_type (out, val) == EXT_LANG_BT_ERROR) - { - retval = EXT_LANG_BT_ERROR; - do_cleanups (cleanups); - } + retval = EXT_LANG_BT_ERROR; } if (retval != EXT_LANG_BT_ERROR) @@ -454,8 +452,6 @@ py_print_single_arg (struct ui_out *out, retval = EXT_LANG_BT_ERROR; } } - - do_cleanups (cleanups); } } CATCH (except, RETURN_MASK_ERROR) @@ -695,35 +691,24 @@ enumerate_locals (PyObject *iter, struct symbol *sym; struct block *sym_block; int local_indent = 8 + (8 * indent); - struct cleanup *locals_cleanups; + gdb::optional tuple; gdbpy_ref item (PyIter_Next (iter)); if (item == NULL) break; - locals_cleanups = make_cleanup (null_cleanup, NULL); - success = extract_sym (item.get (), &sym_name, &sym, &sym_block, &language); if (success == EXT_LANG_BT_ERROR) - { - do_cleanups (locals_cleanups); - goto error; - } + return EXT_LANG_BT_ERROR; success = extract_value (item.get (), &val); if (success == EXT_LANG_BT_ERROR) - { - do_cleanups (locals_cleanups); - goto error; - } + return EXT_LANG_BT_ERROR; if (sym != NULL && out->is_mi_like_p () && ! mi_should_print (sym, MI_PRINT_LOCALS)) - { - do_cleanups (locals_cleanups); - continue; - } + continue; /* If the object did not provide a value, read it. */ if (val == NULL) @@ -735,8 +720,7 @@ enumerate_locals (PyObject *iter, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (locals_cleanups); - goto error; + return EXT_LANG_BT_ERROR; } END_CATCH } @@ -747,7 +731,7 @@ enumerate_locals (PyObject *iter, if (out->is_mi_like_p ()) { if (print_args_field || args_type != NO_VALUES) - make_cleanup_ui_out_tuple_begin_end (out, NULL); + tuple.emplace (out, nullptr); } TRY { @@ -765,18 +749,14 @@ enumerate_locals (PyObject *iter, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (locals_cleanups); - goto error; + return EXT_LANG_BT_ERROR; } END_CATCH if (args_type == MI_PRINT_SIMPLE_VALUES) { if (py_print_type (out, val) == EXT_LANG_BT_ERROR) - { - do_cleanups (locals_cleanups); - goto error; - } + return EXT_LANG_BT_ERROR; } /* CLI always prints values for locals. MI uses the @@ -787,10 +767,7 @@ enumerate_locals (PyObject *iter, if (py_print_value (out, val, &opts, val_indent, args_type, language) == EXT_LANG_BT_ERROR) - { - do_cleanups (locals_cleanups); - goto error; - } + return EXT_LANG_BT_ERROR; } else { @@ -798,15 +775,10 @@ enumerate_locals (PyObject *iter, { if (py_print_value (out, val, &opts, 0, args_type, language) == EXT_LANG_BT_ERROR) - { - do_cleanups (locals_cleanups); - goto error; - } + return EXT_LANG_BT_ERROR; } } - do_cleanups (locals_cleanups); - TRY { out->text ("\n"); @@ -814,7 +786,7 @@ enumerate_locals (PyObject *iter, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - goto error; + return EXT_LANG_BT_ERROR; } END_CATCH } @@ -822,7 +794,6 @@ enumerate_locals (PyObject *iter, if (!PyErr_Occurred ()) return EXT_LANG_BT_OK; - error: return EXT_LANG_BT_ERROR; } @@ -835,8 +806,6 @@ py_mi_print_variables (PyObject *filter, struct ui_out *out, enum ext_lang_frame_args args_type, struct frame_info *frame) { - struct cleanup *old_chain; - gdbpy_ref args_iter (get_py_iter_from_func (filter, "frame_args")); if (args_iter == NULL) return EXT_LANG_BT_ERROR; @@ -845,24 +814,19 @@ py_mi_print_variables (PyObject *filter, struct ui_out *out, if (locals_iter == NULL) return EXT_LANG_BT_ERROR; - old_chain = make_cleanup_ui_out_list_begin_end (out, "variables"); + ui_out_emit_list list_emitter (out, "variables"); - if (args_iter != Py_None) - if (enumerate_args (args_iter.get (), out, args_type, 1, frame) - == EXT_LANG_BT_ERROR) - goto error; + if (args_iter != Py_None + && (enumerate_args (args_iter.get (), out, args_type, 1, frame) + == EXT_LANG_BT_ERROR)) + return EXT_LANG_BT_ERROR; - if (locals_iter != Py_None) - if (enumerate_locals (locals_iter.get (), out, 1, args_type, 1, frame) - == EXT_LANG_BT_ERROR) - goto error; + if (locals_iter != Py_None + && (enumerate_locals (locals_iter.get (), out, 1, args_type, 1, frame) + == EXT_LANG_BT_ERROR)) + return EXT_LANG_BT_ERROR; - do_cleanups (old_chain); return EXT_LANG_BT_OK; - - error: - do_cleanups (old_chain); - return EXT_LANG_BT_ERROR; } /* Helper function for printing locals. This function largely just @@ -876,25 +840,18 @@ py_print_locals (PyObject *filter, int indent, struct frame_info *frame) { - struct cleanup *old_chain; - gdbpy_ref locals_iter (get_py_iter_from_func (filter, "frame_locals")); if (locals_iter == NULL) return EXT_LANG_BT_ERROR; - old_chain = make_cleanup_ui_out_list_begin_end (out, "locals"); + ui_out_emit_list list_emitter (out, "locals"); - if (locals_iter != Py_None) - if (enumerate_locals (locals_iter.get (), out, indent, args_type, - 0, frame) == EXT_LANG_BT_ERROR) - goto locals_error; + if (locals_iter != Py_None + && (enumerate_locals (locals_iter.get (), out, indent, args_type, + 0, frame) == EXT_LANG_BT_ERROR)) + return EXT_LANG_BT_ERROR; - do_cleanups (old_chain); return EXT_LANG_BT_OK; - - locals_error: - do_cleanups (old_chain); - return EXT_LANG_BT_ERROR; } /* Helper function for printing frame arguments. This function @@ -908,13 +865,11 @@ py_print_args (PyObject *filter, enum ext_lang_frame_args args_type, struct frame_info *frame) { - struct cleanup *old_chain; - gdbpy_ref args_iter (get_py_iter_from_func (filter, "frame_args")); if (args_iter == NULL) return EXT_LANG_BT_ERROR; - old_chain = make_cleanup_ui_out_list_begin_end (out, "args"); + ui_out_emit_list list_emitter (out, "args"); TRY { @@ -925,14 +880,14 @@ py_print_args (PyObject *filter, CATCH (except, RETURN_MASK_ALL) { gdbpy_convert_exception (except); - goto args_error; + return EXT_LANG_BT_ERROR; } END_CATCH - if (args_iter != Py_None) - if (enumerate_args (args_iter.get (), out, args_type, 0, frame) - == EXT_LANG_BT_ERROR) - goto args_error; + if (args_iter != Py_None + && (enumerate_args (args_iter.get (), out, args_type, 0, frame) + == EXT_LANG_BT_ERROR)) + return EXT_LANG_BT_ERROR; TRY { @@ -942,16 +897,11 @@ py_print_args (PyObject *filter, CATCH (except, RETURN_MASK_ALL) { gdbpy_convert_exception (except); - goto args_error; + return EXT_LANG_BT_ERROR; } END_CATCH - do_cleanups (old_chain); return EXT_LANG_BT_OK; - - args_error: - do_cleanups (old_chain); - return EXT_LANG_BT_ERROR; } /* Print a single frame to the designated output stream, detecting @@ -978,7 +928,6 @@ py_print_frame (PyObject *filter, int flags, CORE_ADDR address = 0; struct gdbarch *gdbarch = NULL; struct frame_info *frame = NULL; - struct cleanup *cleanup_stack; struct value_print_options opts; int print_level, print_frame_info, print_args, print_locals; gdb::unique_xmalloc_ptr function_to_free; @@ -1022,12 +971,12 @@ py_print_frame (PyObject *filter, int flags, return EXT_LANG_BT_COMPLETED; } - cleanup_stack = make_cleanup (null_cleanup, NULL); + gdb::optional tuple; /* -stack-list-locals does not require a wrapping frame attribute. */ if (print_frame_info || (print_args && ! print_locals)) - make_cleanup_ui_out_tuple_begin_end (out, "frame"); + tuple.emplace (out, "frame"); if (print_frame_info) { @@ -1042,7 +991,6 @@ py_print_frame (PyObject *filter, int flags, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } END_CATCH @@ -1055,18 +1003,12 @@ py_print_frame (PyObject *filter, int flags, gdbpy_ref paddr (PyObject_CallMethod (filter, "address", NULL)); if (paddr == NULL) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; if (paddr != Py_None) { if (get_addr_from_python (paddr.get (), &address) < 0) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; has_addr = 1; } @@ -1105,7 +1047,6 @@ py_print_frame (PyObject *filter, int flags, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } END_CATCH @@ -1127,7 +1068,6 @@ py_print_frame (PyObject *filter, int flags, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } END_CATCH @@ -1140,20 +1080,14 @@ py_print_frame (PyObject *filter, int flags, const char *function = NULL; if (py_func == NULL) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; if (gdbpy_is_string (py_func.get ())) { function_to_free = python_string_to_host_string (py_func.get ()); if (function_to_free == NULL) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; function = function_to_free.get (); } @@ -1163,10 +1097,7 @@ py_print_frame (PyObject *filter, int flags, struct bound_minimal_symbol msymbol; if (get_addr_from_python (py_func.get (), &addr) < 0) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; msymbol = lookup_minimal_symbol_by_pc (addr); if (msymbol.minsym != NULL) @@ -1177,7 +1108,6 @@ py_print_frame (PyObject *filter, int flags, PyErr_SetString (PyExc_RuntimeError, _("FrameDecorator.function: expecting a " \ "String, integer or None.")); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } @@ -1192,7 +1122,6 @@ py_print_frame (PyObject *filter, int flags, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } END_CATCH @@ -1205,10 +1134,7 @@ py_print_frame (PyObject *filter, int flags, if (print_args) { if (py_print_args (filter, out, args_type, frame) == EXT_LANG_BT_ERROR) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; } /* File name/source/line number information. */ @@ -1221,7 +1147,6 @@ py_print_frame (PyObject *filter, int flags, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } END_CATCH @@ -1231,10 +1156,7 @@ py_print_frame (PyObject *filter, int flags, gdbpy_ref py_fn (PyObject_CallMethod (filter, "filename", NULL)); if (py_fn == NULL) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; if (py_fn != Py_None) { @@ -1242,10 +1164,7 @@ py_print_frame (PyObject *filter, int flags, filename (python_string_to_host_string (py_fn.get ())); if (filename == NULL) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; TRY { @@ -1258,7 +1177,6 @@ py_print_frame (PyObject *filter, int flags, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } END_CATCH @@ -1271,19 +1189,13 @@ py_print_frame (PyObject *filter, int flags, int line; if (py_line == NULL) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; if (py_line != Py_None) { line = PyLong_AsLong (py_line.get ()); if (PyErr_Occurred ()) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; TRY { @@ -1294,7 +1206,6 @@ py_print_frame (PyObject *filter, int flags, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } END_CATCH @@ -1314,7 +1225,6 @@ py_print_frame (PyObject *filter, int flags, CATCH (except, RETURN_MASK_ERROR) { gdbpy_convert_exception (except); - do_cleanups (cleanup_stack); return EXT_LANG_BT_ERROR; } END_CATCH @@ -1324,26 +1234,20 @@ py_print_frame (PyObject *filter, int flags, { if (py_print_locals (filter, out, args_type, indent, frame) == EXT_LANG_BT_ERROR) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; } { /* Finally recursively print elided frames, if any. */ gdbpy_ref elided (get_py_iter_from_func (filter, "elided")); if (elided == NULL) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; if (elided != Py_None) { PyObject *item; - make_cleanup_ui_out_list_begin_end (out, "children"); + ui_out_emit_list inner_list_emiter (out, "children"); if (! out->is_mi_like_p ()) indent++; @@ -1358,20 +1262,13 @@ py_print_frame (PyObject *filter, int flags, levels_printed); if (success == EXT_LANG_BT_ERROR) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; } if (item == NULL && PyErr_Occurred ()) - { - do_cleanups (cleanup_stack); - return EXT_LANG_BT_ERROR; - } + return EXT_LANG_BT_ERROR; } } - do_cleanups (cleanup_stack); return EXT_LANG_BT_COMPLETED; } diff --git a/gdb/ui-out.h b/gdb/ui-out.h index d54843db7be..9278cabdaab 100644 --- a/gdb/ui-out.h +++ b/gdb/ui-out.h @@ -187,4 +187,37 @@ class ui_out ui_out_level *current_level () const; }; +/* This is similar to make_cleanup_ui_out_tuple_begin_end and + make_cleanup_ui_out_list_begin_end, but written as an RAII template + class. It takes the ui_out_type as a template parameter. Normally + this is used via the typedefs ui_out_emit_tuple and + ui_out_emit_list. */ +template +class ui_out_emit_type +{ +public: + + ui_out_emit_type (struct ui_out *uiout, const char *id) + : m_uiout (uiout) + { + uiout->begin (Type, id); + } + + ~ui_out_emit_type () + { + m_uiout->end (Type); + } + + ui_out_emit_type (const ui_out_emit_type &) = delete; + ui_out_emit_type &operator= (const ui_out_emit_type &) + = delete; + +private: + + struct ui_out *m_uiout; +}; + +typedef ui_out_emit_type ui_out_emit_tuple; +typedef ui_out_emit_type ui_out_emit_list; + #endif /* UI_OUT_H */ -- 2.30.2