[PATCH 00/12] remove some cleanups using a cleanup function

Andrew Burgess andrew.burgess@embecosm.com
Mon Jan 21 12:19:00 GMT 2019


* Pedro Alves <palves@redhat.com> [2019-01-16 23:10:23 +0000]:

> On 01/15/2019 11:43 PM, Pedro Alves wrote:
> > On 01/15/2019 11:03 PM, Tom Tromey wrote:
> >>>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> >>
> >> Andrew> Maybe there's some other reason why scoped_finish_thread_state is
> >> Andrew> different, in which case I apologise for wasting everyone's time, but
> >> Andrew> right now it appears to me that scoped_finish_thread_state is no
> >> Andrew> different to cleanup_function, it's just used more.
> >>
> >> FWIW I don't think it's a waste of time at all.  There's no particular
> >> rush for these patches and I think it's more valuable for us to agree on
> >> what we'd like the result to look like than it is to land them quickly.
> > 
> > Definitely agreed.  Not a waste at all!
> > 
> > I've been playing with this today, and I have a different
> > implementation of Andrew's class that allows writing:
> > 
> >  using delete_longjmp_breakpoint_cleanup
> >    = forward_cleanup_function<decltype (delete_longjmp_breakpoint),
> >                               delete_longjmp_breakpoint>;
> > 
> > Or, with a macro to eliminate redundancy:
> > 
> >  using delete_longjmp_breakpoint_cleanup
> >    = FORWARD_CLEANUP_FUNCTION (delete_longjmp_breakpoint);
> > 
> > Naming up in the air, I just picked that as straw man.
> 
> See patch below.  Since cleanup_function turned into scope_exit,
> this new class became forward_scope_exit, because it's a "forwarding"
> version of scope_exit?  I'm really not that sure about that name...
> wrap_scope_exit came to mind too.  Or maybe call it cleanup_function?
> 
> This version builds on top of std::bind, which eliminates the
> need for manually storing the args in the tuple and the recursive
> call_function_on_tuple template code.  As shown above, it also
> doesn't require manually passing the wrapped-function's parameter
> types to the template, they're extracted automatically.
> 
> > 
> >>
> >> Andrew> I think if we're going to put in a generic solution (which I think is
> >> Andrew> a good thing) then we should either, make sure we understand why
> >> Andrew> scoped_finish_thread_state is different (and what the rules are for
> >> Andrew> when to use the generic, and when to create a class), or, make sure
> >> Andrew> the generic is suitable to replace scoped_finish_thread_state.
> >>
> >> Andrew> (I'm not trying to pick on scoped_finish_thread_state, it was just the
> >> Andrew> first example I found when originally replying to Tom.)
> >>
> >> Maybe I was just making too big a deal out of it, but my thinking was
> >> that writing the finish_thread_state call at each spot would be bad,
> >> since it would be multiple copies of the same thing.  But, maybe it is
> >> actually no big deal?
> 
> Yeah, I think that writing
> 
>  SCOPE_EXIT { finish_thread_state (ptid); };
> 
> in the multiple places wouldn't be a big deal.
> 
> However, I found that Andrew's idea is most useful for the
> gdb::optional case, since in that case we need the cleanup's type
> upfront.  The finish_thread_state cleanup is in that situation,
> so I converted it to a forward_scope_exit.
> 
> >>
> >> Using a template, as Pedro suggested, would remove some of the ugliness
> >> from the series.  Stuff like this:
> >>
> >> +  auto do_invalidate
> >> +    = [=] ()
> >> +      {
> >> +	this->invalidate (regnum);
> >> +      };
> >> +  cleanup_function invalidator (do_invalidate);
> >>
> >> Could instead just be:
> >>
> >>   SCOPE_EXIT { this->invalidate (regnum); }
> >>
> >> ... assuming we like SCOPE_EXIT (to me it seems reasonable enough).
> 
> In this particular case, it can't be SCOPE_EXIT because we need
> to be able to cancel the scope, i.e., we need to scope_exit's name.
> So we end up with:
> 
>   auto invalidator
>     = make_scope_exit ([&] { this->invalidate (regnum); });
> 
> I guess we should add an alternative macro like to be used like:
> 
>   auto invalidator 
>    = NAMED_SCOPE_EXIT { this->invalidate (regnum); };
> 
> I didn't do that though.  Doesn't save that much?
> 
> Ideally we'd find a way to implement SCOPE_FAILURE/SCOPE_SUCCESS
> in C++11 instead...
> 
> >>
> >> Anyway, I tend to think we should simply copy the scope_exit paper.  If
> >> it's accepted into C++ then someday we can just remove the gdb variant.
> >>
> >> Let me know if you agree; if so I can implement this.
> >>
> > I've also played with the template idea, basically implemented
> > scope_exit / make_scope_exit.  Seems to work nicely.
> 
> I did this more fully today, following the description in the paper.
> 
> I think this is the first time I had found a use for a function-try-block,
> see scope_exit ctor...
> 
> > 
> > I hadn't done the SCOPE_EXIT macro though, not sure it is worth
> > it to have yet another way to write these things (thinking about
> > newcomers' cognitive load, having to learn all the different
> > things) -- of all the make_scope_exit calls I have, most either
> > take the form:
> > 
> >   auto cleanup = make_scope_exit (function);
> > 
> > i.e., are passed a function pointer, can do without a
> > lambda, and/or need access to the scope_exit object to
> > cancel it.  But I can give it a try for sure.  It may
> > be clearer code to standardize writing:
> > 
> >   auto cleanup = make_scope_exit (function);
> >   cleanup.release ();
> > 
> > when the cleanup may need to be canceled and
> > 
> >   SCOPE_EXIT { function (); }
> > 
> > when it doesn't?
> 
> I did this.
> 
> > 
> > I won't be able to finish this today (I'd like to clean up a couple hacks
> > here and there), but I'll post something tomorrow so we can all see
> > and decide a way forward.
> 
> I remembered struct on_exit I had added to gdbarch-selftests.c,
> thinking of a generic SCOPE_EXIT at the time.  :-)  So I converted
> that too now.
> 
> This revealed that the ESC macro in common/preprocessor.h conflicts
> with a macro of the same name in readline.h...  That should go in
> a separate patch.
> 
> Since both scope_exit and forward_scope_exit share some
> functionality, I put the shared code/structure in
> a common base class, using CRTP.
> 
> Here is patch with everything together.  WDYT?

I think this patch looks good.

Thanks for taking this on.

Andrew


> 
> From c616fefa268155eea243dedfb6ff54e133ee33f9 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 16 Jan 2019 20:45:52 +0000
> Subject: [PATCH] scope_exit
> 
> ---
>  gdb/breakpoint.c                |  27 ++---
>  gdb/common/forward-scope-exit.h | 118 +++++++++++++++++++++
>  gdb/common/preprocessor.h       |   2 +-
>  gdb/common/scope-exit.h         | 185 ++++++++++++++++++++++++++++++++
>  gdb/common/valid-expr.h         |  18 ++--
>  gdb/gdbarch-selftests.c         |   8 +-
>  gdb/gdbthread.h                 |  28 +----
>  gdb/infcall.c                   |  13 +--
>  gdb/infcmd.c                    |  13 +--
>  gdb/inferior.h                  |   4 +-
>  gdb/infrun.c                    | 230 ++++++++++++++++++----------------------
>  gdb/linux-nat.c                 |  18 +---
>  gdb/regcache.c                  |  34 +-----
>  gdb/symfile.c                   |  27 +++--
>  gdb/top.c                       |   8 +-
>  gdb/ui-out.h                    |   8 +-
>  gdb/utils.c                     |  17 ---
>  gdb/utils.h                     |   1 -
>  18 files changed, 463 insertions(+), 296 deletions(-)
>  create mode 100644 gdb/common/forward-scope-exit.h
>  create mode 100644 gdb/common/scope-exit.h
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 2ab8a76326..7f6cfc4828 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -83,6 +83,7 @@
>  #include "progspace-and-thread.h"
>  #include "common/array-view.h"
>  #include "common/gdb_optional.h"
> +#include "common/scope-exit.h"
>  
>  /* Enums for exception-handling support.  */
>  enum exception_event_kind
> @@ -4468,7 +4469,7 @@ get_bpstat_thread ()
>  void
>  bpstat_do_actions (void)
>  {
> -  struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
> +  auto cleanup_if_error = make_scope_exit (bpstat_clear_actions);
>    thread_info *tp;
>  
>    /* Do any commands attached to breakpoint we are stopped at.  */
> @@ -4482,7 +4483,7 @@ bpstat_do_actions (void)
>  	break;
>      }
>  
> -  discard_cleanups (cleanup_if_error);
> +  cleanup_if_error.release ();
>  }
>  
>  /* Print out the (old or new) value associated with a watchpoint.  */
> @@ -9230,7 +9231,6 @@ create_breakpoint (struct gdbarch *gdbarch,
>  		   unsigned flags)
>  {
>    struct linespec_result canonical;
> -  struct cleanup *bkpt_chain = NULL;
>    int pending = 0;
>    int task = 0;
>    int prev_bkpt_count = breakpoint_count;
> @@ -9280,12 +9280,6 @@ create_breakpoint (struct gdbarch *gdbarch,
>    if (!pending && canonical.lsals.empty ())
>      return 0;
>  
> -  /* ----------------------------- SNIP -----------------------------
> -     Anything added to the cleanup chain beyond this point is assumed
> -     to be part of a breakpoint.  If the breakpoint create succeeds
> -     then the memory is not reclaimed.  */
> -  bkpt_chain = make_cleanup (null_cleanup, 0);
> -
>    /* Resolve all line numbers to PC's and verify that the addresses
>       are ok for the target.  */
>    if (!pending)
> @@ -9384,10 +9378,6 @@ create_breakpoint (struct gdbarch *gdbarch,
>        prev_breakpoint_count = prev_bkpt_count;
>      }
>  
> -  /* That's it.  Discard the cleanups for data inserted into the
> -     breakpoint.  */
> -  discard_cleanups (bkpt_chain);
> -
>    /* error call may happen here - have BKPT_CHAIN already discarded.  */
>    update_global_location_list (UGLL_MAY_INSERT);
>  
> @@ -11073,7 +11063,6 @@ until_break_command (const char *arg, int from_tty, int anywhere)
>    struct gdbarch *frame_gdbarch;
>    struct frame_id stack_frame_id;
>    struct frame_id caller_frame_id;
> -  struct cleanup *old_chain;
>    int thread;
>    struct thread_info *tp;
>    struct until_break_fsm *sm;
> @@ -11106,8 +11095,6 @@ until_break_command (const char *arg, int from_tty, int anywhere)
>    tp = inferior_thread ();
>    thread = tp->global_num;
>  
> -  old_chain = make_cleanup (null_cleanup, NULL);
> -
>    /* Note linespec handling above invalidates the frame chain.
>       Installing a breakpoint also invalidates the frame chain (as it
>       may need to switch threads), so do any frame handling before
> @@ -11122,6 +11109,9 @@ until_break_command (const char *arg, int from_tty, int anywhere)
>       one.  */
>  
>    breakpoint_up caller_breakpoint;
> +
> +  gdb::optional<delete_longjmp_breakpoint_cleanup> lj_deleter;
> +
>    if (frame_id_p (caller_frame_id))
>      {
>        struct symtab_and_line sal2;
> @@ -11136,7 +11126,7 @@ until_break_command (const char *arg, int from_tty, int anywhere)
>  						    bp_until);
>  
>        set_longjmp_breakpoint (tp, caller_frame_id);
> -      make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
> +      lj_deleter.emplace (thread);
>      }
>  
>    /* set_momentary_breakpoint could invalidate FRAME.  */
> @@ -11159,7 +11149,8 @@ until_break_command (const char *arg, int from_tty, int anywhere)
>  			    std::move (caller_breakpoint));
>    tp->thread_fsm = &sm->thread_fsm;
>  
> -  discard_cleanups (old_chain);
> +  if (lj_deleter)
> +    lj_deleter->release ();
>  
>    proceed (-1, GDB_SIGNAL_DEFAULT);
>  }
> diff --git a/gdb/common/forward-scope-exit.h b/gdb/common/forward-scope-exit.h
> new file mode 100644
> index 0000000000..6b51e296b6
> --- /dev/null
> +++ b/gdb/common/forward-scope-exit.h
> @@ -0,0 +1,118 @@
> +/* Copyright (C) 2019 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 <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef COMMON_FORWARD_SCOPE_EXIT_H
> +#define COMMON_FORWARD_SCOPE_EXIT_H
> +
> +#include "common/scope-exit.h"
> +
> +/* A forward_scope_exit is like scope_exit, but instead of giving it a
> +   callable, you instead specialize it for a given cleanup function,
> +   and the generated class automatically has a constructor with the
> +   same interface as the cleanup function.  forward_scope_exit
> +   captures the arguments passed to the ctor, and in turn passes those
> +   as arguments to the wrapped cleanup function, when it is called at
> +   scope exit time, from within the forward_scope_exit dtor.  The
> +   forward_scope_exit class can take any number of arguments, and is
> +   cancelable if needed.
> +
> +   This allows usage like this:
> +
> +      void
> +      delete_longjmp_breakpoint (int arg)
> +      {
> +	// Blah, blah, blah...
> +      }
> +
> +      using longjmp_breakpoint_cleanup
> +	= FORWARD_SCOPE_EXIT (delete_longjmp_breakpoint);
> +
> +   This above created a new cleanup class `longjmp_breakpoint_cleanup`
> +   than can then be used like this:
> +
> +      longjmp_breakpoint_cleanup obj (thread);
> +
> +      // Blah, blah, blah...
> +
> +      obj.release ();  // Optional cancel if needed.
> +
> +   forward_scope_exit is also handy when you would need to wrap a
> +   scope_exit in a gdb::optional:
> +
> +      gdb::optional<longjmp_breakpoint_cleanup> cleanup;
> +      if (some condition)
> +	cleanup.emplace (thread);
> +      ...
> +      if (cleanup)
> +	cleanup->release ();
> +
> +   since with scope exit, you would have to know the scope_exit's
> +   callable template type when you create the gdb::optional:
> +
> +     gdb:optional<scope_exit<what goes here?>>
> +*/
> +
> +namespace detail
> +{
> +
> +/* Function and Signature are passed in the same type, in order to
> +   extract Function's arguments' types in the specialization below.
> +   Those are used to generate the constructor.  */
> +
> +template<typename Function, Function *function, typename Signature>
> +struct forward_scope_exit;
> +
> +template<typename Function, Function *function,
> +	 typename Res, typename... Args>
> +class forward_scope_exit<Function, function, Res (Args...)>
> +  : public scope_exit_base<forward_scope_exit<Function,
> +					      function,
> +					      Res (Args...)>>
> +{
> +  /* For access to on_exit().  */
> +  friend scope_exit_base<forward_scope_exit<Function,
> +					    function,
> +					    Res (Args...)>>;
> +
> +public:
> +  explicit forward_scope_exit (Args ...args)
> +    : m_bind_function (std::bind (function, args...))
> +  {
> +    /* Nothing.  */
> +  }
> +
> +private:
> +  void on_exit ()
> +  {
> +    m_bind_function ();
> +  }
> +
> +  /* The function and the arguments passed to the ctor, all packed in
> +     a std::bind.  */
> +  decltype (std::bind (function, std::declval<Args> ()...))
> +    m_bind_function;
> +};
> +
> +} /* namespace detail */
> +
> +/* This is the "public" entry point.  It's a macro to avoid having to
> +   name FUNC more than once.  */
> +
> +#define FORWARD_SCOPE_EXIT(FUNC) \
> +  detail::forward_scope_exit<decltype (FUNC), FUNC, decltype (FUNC)>
> +
> +#endif /* COMMON_FORWARD_SCOPE_EXIT_H */
> diff --git a/gdb/common/preprocessor.h b/gdb/common/preprocessor.h
> index 647568ede6..a7c33d0978 100644
> --- a/gdb/common/preprocessor.h
> +++ b/gdb/common/preprocessor.h
> @@ -30,6 +30,6 @@
>  
>  /* Escape parens out.  Useful if you need to pass an argument that
>     includes commas to another macro.  */
> -#define ESC(...) __VA_ARGS__
> +#define ESC_PARENS(...) __VA_ARGS__
>  
>  #endif /* COMMON_PREPROC */
> diff --git a/gdb/common/scope-exit.h b/gdb/common/scope-exit.h
> new file mode 100644
> index 0000000000..a484304372
> --- /dev/null
> +++ b/gdb/common/scope-exit.h
> @@ -0,0 +1,185 @@
> +/* Copyright (C) 2019 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 <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef COMMON_SCOPE_EXIT_H
> +#define COMMON_SCOPE_EXIT_H
> +
> +#include <functional>
> +#include <type_traits>
> +#include "common/preprocessor.h"
> +
> +/* scope_exit is a general-purpose scope guard that calls its exit
> +   function at the end of the current scope.  A scope_exit may be
> +   canceled by calling the "release" method.  The API is modeled on
> +   P0052R5 - Generic Scope Guard and RAII Wrapper for the Standard
> +   Library, which is itself based on Andrej Alexandrescu's
> +   ScopeGuard/SCOPE_EXIT.
> +
> +   There are two forms available:
> +
> +   - The "make_scope_exit" form allows canceling the scope guard.  Use
> +     it like this:
> +
> +     auto cleanup = make_scope_exit ( <function, function object, lambda> );
> +     ...
> +     cleanup.release (); // cancel
> +
> +   - If you don't need to cancel the guard, you can use the SCOPE_EXIT
> +     macro, like this:
> +
> +     SCOPE_EXIT
> +       {
> +	 // any code you like here.
> +       }
> +
> +   See also forward_scope_exit.
> +*/
> +
> +/* CRTP base class for cancelable scope_exit-like classes.  Implements
> +   the common call-custom-function-from-dtor functionality.  Classes
> +   that inherit this implement the on_exit() method, which is called
> +   from scope_exit_base's dtor.  */
> +
> +template <typename CRTP>
> +class scope_exit_base
> +{
> +public:
> +  scope_exit_base () = default;
> +
> +  ~scope_exit_base ()
> +  {
> +    if (!m_released)
> +      {
> +	auto *self = static_cast<CRTP *> (this);
> +	self->on_exit ();
> +      }
> +  }
> +
> +  /* This is needed for make_scope_exit because copy elision isn't
> +     guaranteed until C++17.  An optimizing compiler will usually skip
> +     calling this, but it must exist.  */
> +  scope_exit_base (const scope_exit_base &other)
> +    : m_released (other.m_released)
> +  {
> +    other.m_released = true;
> +  }
> +
> +  void operator= (const scope_exit_base &) = delete;
> +
> +  /* If this is called, then the wrapped function will not be called
> +     on destruction.  */
> +  void release () noexcept
> +  {
> +    m_released = true;
> +  }
> +
> +private:
> +
> +  /* True if released.  Mutable because of the copy ctor hack
> +     above.  */
> +  mutable bool m_released = false;
> +};
> +
> +/* A cleanup function is one that is run at the end of the current
> +   scope.  A cleanup function may be canceled by calling the "cancel"
> +   method.  */
> +
> +template<typename EF>
> +class scope_exit : public scope_exit_base<scope_exit<EF>>
> +{
> +  /* For access to on_exit().  */
> +  friend scope_exit_base<scope_exit<EF>>;
> +
> +public:
> +
> +  template<typename EFP,
> +	   typename = gdb::Requires<std::is_constructible<EF, EFP>>>
> +  scope_exit (EFP &&f)
> +    try : m_exit_function ((!std::is_lvalue_reference<EFP>::value
> +			    && std::is_nothrow_constructible<EF, EFP>::value)
> +			   ? std::move (f)
> +			   : f)
> +  {
> +  }
> +  catch (...)
> +    {
> +      /* "If the initialization of exit_function throws an exception,
> +	 calls f()."  */
> +      f ();
> +    }
> +
> +  template<typename EFP,
> +	   typename = gdb::Requires<std::is_constructible<EF, EFP>>>
> +  scope_exit (scope_exit &&rhs)
> +    noexcept (std::is_nothrow_move_constructible<EF>::value
> +	      || std::is_nothrow_copy_constructible<EF>::value)
> +    : m_exit_function (std::is_nothrow_constructible<EFP>::value
> +		       ? std::move (rhs)
> +		       : rhs)
> +  {
> +    rhs.release ();
> +  }
> +
> +  /* This is needed for make_scope_exit because copy elision isn't
> +     guaranteed until C++17.  An optimizing compiler will usually skip
> +     calling this, but it must exist.  */
> +  scope_exit (const scope_exit &other)
> +    : scope_exit_base<scope_exit<EF>> (other),
> +      m_exit_function (other.m_exit_function)
> +  {
> +  }
> +
> +  void operator= (const scope_exit &) = delete;
> +  void operator= (scope_exit &&) = delete;
> +
> +private:
> +  void on_exit ()
> +  {
> +    m_exit_function ();
> +  }
> +
> +  /* The function to call on scope exit.  */
> +  EF m_exit_function;
> +};
> +
> +template <typename EF>
> +scope_exit<typename std::decay<EF>::type>
> +make_scope_exit (EF &&f)
> +{
> +  return scope_exit<typename std::decay<EF>::type> (std::forward<EF> (f));
> +}
> +
> +namespace detail
> +{
> +
> +enum class scope_exit_lhs {};
> +
> +template<typename EF>
> +scope_exit<typename std::decay<EF>::type>
> +operator+ (scope_exit_lhs, EF &&rhs)
> +{
> +  return scope_exit<typename std::decay<EF>::type> (std::forward<EF> (rhs));
> +}
> +
> +}
> +
> +/* Register a block of code to run on scope exit.  */
> +
> +#define SCOPE_EXIT \
> +  auto CONCAT(scope_exit_, __LINE__) = ::detail::scope_exit_lhs () + [&] ()
> +
> +#endif /* COMMON_SCOPE_EXIT_H */
> diff --git a/gdb/common/valid-expr.h b/gdb/common/valid-expr.h
> index 9289448365..603c76e8f1 100644
> --- a/gdb/common/valid-expr.h
> +++ b/gdb/common/valid-expr.h
> @@ -85,24 +85,24 @@
>     another variant.  */
>  
>  #define CHECK_VALID_EXPR_1(T1, VALID, EXPR_TYPE, EXPR)			\
> -  CHECK_VALID_EXPR_INT (ESC (typename T1),				\
> -			ESC (T1),					\
> +  CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1),			\
> +			ESC_PARENS (T1),				\
>  			VALID, EXPR_TYPE, EXPR)
>  
>  #define CHECK_VALID_EXPR_2(T1, T2, VALID, EXPR_TYPE, EXPR)		\
> -  CHECK_VALID_EXPR_INT (ESC (typename T1, typename T2),			\
> -			ESC (T1, T2),					\
> +  CHECK_VALID_EXPR_INT (ESC_PARENS(typename T1, typename T2),		\
> +			ESC_PARENS (T1, T2),				\
>  			VALID, EXPR_TYPE, EXPR)
>  
>  #define CHECK_VALID_EXPR_3(T1, T2, T3, VALID, EXPR_TYPE, EXPR)		\
> -  CHECK_VALID_EXPR_INT (ESC (typename T1, typename T2, typename T3),	\
> -			ESC (T1, T2, T3),				\
> +  CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1, typename T2, typename T3), \
> +			ESC_PARENS (T1, T2, T3),				\
>  			VALID, EXPR_TYPE, EXPR)
>  
>  #define CHECK_VALID_EXPR_4(T1, T2, T3, T4, VALID, EXPR_TYPE, EXPR)	\
> -  CHECK_VALID_EXPR_INT (ESC (typename T1, typename T2,			\
> -			     typename T3, typename T4),			\
> -			ESC (T1, T2, T3, T4),				\
> +  CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1, typename T2,		\
> +				    typename T3, typename T4),		\
> +			ESC_PARENS (T1, T2, T3, T4),			\
>  			VALID, EXPR_TYPE, EXPR)
>  
>  #endif /* COMMON_VALID_EXPR_H */
> diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
> index 50062abe8a..64d1e543e4 100644
> --- a/gdb/gdbarch-selftests.c
> +++ b/gdb/gdbarch-selftests.c
> @@ -104,13 +104,7 @@ register_to_value_test (struct gdbarch *gdbarch)
>    push_target (&mock_target);
>  
>    /* Pop it again on exit (return/exception).  */
> -  struct on_exit
> -  {
> -    ~on_exit ()
> -    {
> -      pop_all_targets_at_and_above (process_stratum);
> -    }
> -  } pop_targets;
> +  SCOPE_EXIT { pop_all_targets_at_and_above (process_stratum); };
>  
>    /* Switch to the mock thread.  */
>    scoped_restore restore_inferior_ptid
> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index 95db69605e..c35a54e75d 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -32,6 +32,7 @@ struct symtab;
>  #include "cli/cli-utils.h"
>  #include "common/refcounted-object.h"
>  #include "common-gdbthread.h"
> +#include "common/forward-scope-exit.h"
>  
>  struct inferior;
>  
> @@ -612,31 +613,8 @@ extern void finish_thread_state (ptid_t ptid);
>  
>  /* Calls finish_thread_state on scope exit, unless release() is called
>     to disengage.  */
> -class scoped_finish_thread_state
> -{
> -public:
> -  explicit scoped_finish_thread_state (ptid_t ptid)
> -    : m_ptid (ptid)
> -  {}
> -
> -  ~scoped_finish_thread_state ()
> -  {
> -    if (!m_released)
> -      finish_thread_state (m_ptid);
> -  }
> -
> -  /* Disengage.  */
> -  void release ()
> -  {
> -    m_released = true;
> -  }
> -
> -  DISABLE_COPY_AND_ASSIGN (scoped_finish_thread_state);
> -
> -private:
> -  bool m_released = false;
> -  ptid_t m_ptid;
> -};
> +using scoped_finish_thread_state
> +  = FORWARD_SCOPE_EXIT (finish_thread_state);
>  
>  /* Commands with a prefix of `thread'.  */
>  extern struct cmd_list_element *thread_cmd_list;
> diff --git a/gdb/infcall.c b/gdb/infcall.c
> index 14b0cbc716..6ca479ac1f 100644
> --- a/gdb/infcall.c
> +++ b/gdb/infcall.c
> @@ -40,6 +40,7 @@
>  #include "interps.h"
>  #include "thread-fsm.h"
>  #include <algorithm>
> +#include "common/scope-exit.h"
>  
>  /* If we can't find a function's name from its address,
>     we print this instead.  */
> @@ -675,13 +676,6 @@ run_inferior_call (struct call_thread_fsm *sm,
>    return caught_error;
>  }
>  
> -/* A cleanup function that calls delete_std_terminate_breakpoint.  */
> -static void
> -cleanup_delete_std_terminate_breakpoint (void *ignore)
> -{
> -  delete_std_terminate_breakpoint ();
> -}
> -
>  /* See infcall.h.  */
>  
>  struct value *
> @@ -727,7 +721,6 @@ call_function_by_hand_dummy (struct value *function,
>    struct frame_id dummy_id;
>    struct frame_info *frame;
>    struct gdbarch *gdbarch;
> -  struct cleanup *terminate_bp_cleanup;
>    ptid_t call_thread_ptid;
>    struct gdb_exception e;
>    char name_buf[RAW_FUNCTION_ADDRESS_SIZE];
> @@ -1122,8 +1115,7 @@ call_function_by_hand_dummy (struct value *function,
>  			       dummy_dtor, dummy_dtor_data);
>  
>    /* Register a clean-up for unwind_on_terminating_exception_breakpoint.  */
> -  terminate_bp_cleanup = make_cleanup (cleanup_delete_std_terminate_breakpoint,
> -				       NULL);
> +  SCOPE_EXIT { delete_std_terminate_breakpoint (); };
>  
>    /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP -
>       If you're looking to implement asynchronous dummy-frames, then
> @@ -1184,7 +1176,6 @@ call_function_by_hand_dummy (struct value *function,
>  
>  	    maybe_remove_breakpoints ();
>  
> -	    do_cleanups (terminate_bp_cleanup);
>  	    gdb_assert (retval != NULL);
>  	    return retval;
>  	  }
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 3c3add89ab..a75388c0af 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -59,6 +59,7 @@
>  #include "top.h"
>  #include "interps.h"
>  #include "common/gdb_optional.h"
> +#include "common/scope-exit.h"
>  #include "source.h"
>  
>  /* Local functions: */
> @@ -947,13 +948,6 @@ nexti_command (const char *count_string, int from_tty)
>    step_1 (1, 1, count_string);
>  }
>  
> -void
> -delete_longjmp_breakpoint_cleanup (void *arg)
> -{
> -  int thread = * (int *) arg;
> -  delete_longjmp_breakpoint (thread);
> -}
> -
>  /* Data for the FSM that manages the step/next/stepi/nexti
>     commands.  */
>  
> @@ -1517,7 +1511,6 @@ until_next_command (int from_tty)
>    struct symtab_and_line sal;
>    struct thread_info *tp = inferior_thread ();
>    int thread = tp->global_num;
> -  struct cleanup *old_chain;
>    struct until_next_fsm *sm;
>  
>    clear_proceed_status (0);
> @@ -1556,11 +1549,11 @@ until_next_command (int from_tty)
>    tp->control.step_over_calls = STEP_OVER_ALL;
>  
>    set_longjmp_breakpoint (tp, get_frame_id (frame));
> -  old_chain = make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
> +  delete_longjmp_breakpoint_cleanup lj_deleter (thread);
>  
>    sm = new_until_next_fsm (command_interp (), tp->global_num);
>    tp->thread_fsm = &sm->thread_fsm;
> -  discard_cleanups (old_chain);
> +  lj_deleter.release ();
>  
>    proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
>  }
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index a82df1a52a..6ce21e3ddd 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -51,6 +51,7 @@ struct thread_info;
>  
>  #include "symfile-add-flags.h"
>  #include "common/refcounted-object.h"
> +#include "common/scope-exit.h"
>  
>  #include "common-inferior.h"
>  #include "gdbthread.h"
> @@ -198,7 +199,8 @@ extern void continue_1 (int all_threads);
>  
>  extern void interrupt_target_1 (int all_threads);
>  
> -extern void delete_longjmp_breakpoint_cleanup (void *arg);
> +using delete_longjmp_breakpoint_cleanup
> +  = FORWARD_SCOPE_EXIT (delete_longjmp_breakpoint);
>  
>  extern void detach_command (const char *, int);
>  
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 150288264f..cdfdf49070 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -67,6 +67,7 @@
>  #include "progspace-and-thread.h"
>  #include "common/gdb_optional.h"
>  #include "arch-utils.h"
> +#include "common/scope-exit.h"
>  
>  /* Prototypes for local functions */
>  
> @@ -3247,14 +3248,6 @@ delete_just_stopped_threads_single_step_breakpoints (void)
>    for_each_just_stopped_thread (delete_single_step_breakpoints);
>  }
>  
> -/* A cleanup wrapper.  */
> -
> -static void
> -delete_just_stopped_threads_infrun_breakpoints_cleanup (void *arg)
> -{
> -  delete_just_stopped_threads_infrun_breakpoints ();
> -}
> -
>  /* See infrun.h.  */
>  
>  void
> @@ -3538,15 +3531,11 @@ prepare_for_detach (void)
>  void
>  wait_for_inferior (void)
>  {
> -  struct cleanup *old_cleanups;
> -
>    if (debug_infrun)
>      fprintf_unfiltered
>        (gdb_stdlog, "infrun: wait_for_inferior ()\n");
>  
> -  old_cleanups
> -    = make_cleanup (delete_just_stopped_threads_infrun_breakpoints_cleanup,
> -		    NULL);
> +  SCOPE_EXIT { delete_just_stopped_threads_infrun_breakpoints (); };
>  
>    /* If an error happens while handling the event, propagate GDB's
>       knowledge of the executing state to the frontend/user running
> @@ -3583,8 +3572,6 @@ wait_for_inferior (void)
>  
>    /* No error, don't finish the state yet.  */
>    finish_state.release ();
> -
> -  do_cleanups (old_cleanups);
>  }
>  
>  /* Cleanup that reinstalls the readline callback handler, if the
> @@ -3598,7 +3585,7 @@ wait_for_inferior (void)
>     input.  */
>  
>  static void
> -reinstall_readline_callback_handler_cleanup (void *arg)
> +reinstall_readline_callback_handler_cleanup ()
>  {
>    struct ui *ui = current_ui;
>  
> @@ -3700,7 +3687,6 @@ fetch_inferior_event (void *client_data)
>  {
>    struct execution_control_state ecss;
>    struct execution_control_state *ecs = &ecss;
> -  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
>    int cmd_done = 0;
>    ptid_t waiton_ptid = minus_one_ptid;
>  
> @@ -3712,114 +3698,119 @@ fetch_inferior_event (void *client_data)
>    scoped_restore save_ui = make_scoped_restore (&current_ui, main_ui);
>  
>    /* End up with readline processing input, if necessary.  */
> -  make_cleanup (reinstall_readline_callback_handler_cleanup, NULL);
> -
> -  /* We're handling a live event, so make sure we're doing live
> -     debugging.  If we're looking at traceframes while the target is
> -     running, we're going to need to get back to that mode after
> -     handling the event.  */
> -  gdb::optional<scoped_restore_current_traceframe> maybe_restore_traceframe;
> -  if (non_stop)
> -    {
> -      maybe_restore_traceframe.emplace ();
> -      set_current_traceframe (-1);
> -    }
> -
> -  gdb::optional<scoped_restore_current_thread> maybe_restore_thread;
> -
> -  if (non_stop)
> -    /* In non-stop mode, the user/frontend should not notice a thread
> -       switch due to internal events.  Make sure we reverse to the
> -       user selected thread and frame after handling the event and
> -       running any breakpoint commands.  */
> -    maybe_restore_thread.emplace ();
> -
> -  overlay_cache_invalid = 1;
> -  /* Flush target cache before starting to handle each event.  Target
> -     was running and cache could be stale.  This is just a heuristic.
> -     Running threads may modify target memory, but we don't get any
> -     event.  */
> -  target_dcache_invalidate ();
> -
> -  scoped_restore save_exec_dir
> -    = make_scoped_restore (&execution_direction, target_execution_direction ());
> -
> -  ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
> -			      target_can_async_p () ? TARGET_WNOHANG : 0);
> -
> -  if (debug_infrun)
> -    print_target_wait_results (waiton_ptid, ecs->ptid, &ecs->ws);
> -
> -  /* If an error happens while handling the event, propagate GDB's
> -     knowledge of the executing state to the frontend/user running
> -     state.  */
> -  ptid_t finish_ptid = !target_is_non_stop_p () ? minus_one_ptid : ecs->ptid;
> -  scoped_finish_thread_state finish_state (finish_ptid);
> -
> -  /* Get executed before make_cleanup_restore_current_thread above to apply
> -     still for the thread which has thrown the exception.  */
> -  struct cleanup *ts_old_chain = make_bpstat_clear_actions_cleanup ();
> -
> -  make_cleanup (delete_just_stopped_threads_infrun_breakpoints_cleanup, NULL);
> -
> -  /* Now figure out what to do with the result of the result.  */
> -  handle_inferior_event (ecs);
> +  {
> +    SCOPE_EXIT { reinstall_readline_callback_handler_cleanup (); };
> +
> +    /* We're handling a live event, so make sure we're doing live
> +       debugging.  If we're looking at traceframes while the target is
> +       running, we're going to need to get back to that mode after
> +       handling the event.  */
> +    gdb::optional<scoped_restore_current_traceframe> maybe_restore_traceframe;
> +    if (non_stop)
> +      {
> +	maybe_restore_traceframe.emplace ();
> +	set_current_traceframe (-1);
> +      }
>  
> -  if (!ecs->wait_some_more)
> -    {
> -      struct inferior *inf = find_inferior_ptid (ecs->ptid);
> -      int should_stop = 1;
> -      struct thread_info *thr = ecs->event_thread;
> +    gdb::optional<scoped_restore_current_thread> maybe_restore_thread;
> +
> +    if (non_stop)
> +      /* In non-stop mode, the user/frontend should not notice a thread
> +	 switch due to internal events.  Make sure we reverse to the
> +	 user selected thread and frame after handling the event and
> +	 running any breakpoint commands.  */
> +      maybe_restore_thread.emplace ();
> +
> +    overlay_cache_invalid = 1;
> +    /* Flush target cache before starting to handle each event.  Target
> +       was running and cache could be stale.  This is just a heuristic.
> +       Running threads may modify target memory, but we don't get any
> +       event.  */
> +    target_dcache_invalidate ();
> +
> +    scoped_restore save_exec_dir
> +      = make_scoped_restore (&execution_direction,
> +			     target_execution_direction ());
> +
> +    ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
> +				target_can_async_p () ? TARGET_WNOHANG : 0);
> +
> +    if (debug_infrun)
> +      print_target_wait_results (waiton_ptid, ecs->ptid, &ecs->ws);
> +
> +    /* If an error happens while handling the event, propagate GDB's
> +       knowledge of the executing state to the frontend/user running
> +       state.  */
> +    ptid_t finish_ptid = !target_is_non_stop_p () ? minus_one_ptid : ecs->ptid;
> +    scoped_finish_thread_state finish_state (finish_ptid);
> +
> +    /* Get executed before scoped_restore_current_thread above to apply
> +       still for the thread which has thrown the exception.  */
> +    auto defer_bpstat_clear
> +      = make_scope_exit (bpstat_clear_actions);
> +    auto defer_delete_threads
> +      = make_scope_exit (delete_just_stopped_threads_infrun_breakpoints);
> +
> +    /* Now figure out what to do with the result of the result.  */
> +    handle_inferior_event (ecs);
> +
> +    if (!ecs->wait_some_more)
> +      {
> +	struct inferior *inf = find_inferior_ptid (ecs->ptid);
> +	int should_stop = 1;
> +	struct thread_info *thr = ecs->event_thread;
>  
> -      delete_just_stopped_threads_infrun_breakpoints ();
> +	delete_just_stopped_threads_infrun_breakpoints ();
>  
> -      if (thr != NULL)
> -	{
> -	  struct thread_fsm *thread_fsm = thr->thread_fsm;
> +	if (thr != NULL)
> +	  {
> +	    struct thread_fsm *thread_fsm = thr->thread_fsm;
>  
> -	  if (thread_fsm != NULL)
> -	    should_stop = thread_fsm_should_stop (thread_fsm, thr);
> -	}
> +	    if (thread_fsm != NULL)
> +	      should_stop = thread_fsm_should_stop (thread_fsm, thr);
> +	  }
>  
> -      if (!should_stop)
> -	{
> -	  keep_going (ecs);
> -	}
> -      else
> -	{
> -	  int should_notify_stop = 1;
> -	  int proceeded = 0;
> +	if (!should_stop)
> +	  {
> +	    keep_going (ecs);
> +	  }
> +	else
> +	  {
> +	    int should_notify_stop = 1;
> +	    int proceeded = 0;
>  
> -	  clean_up_just_stopped_threads_fsms (ecs);
> +	    clean_up_just_stopped_threads_fsms (ecs);
>  
> -	  if (thr != NULL && thr->thread_fsm != NULL)
> -	    {
> -	      should_notify_stop
> -		= thread_fsm_should_notify_stop (thr->thread_fsm);
> -	    }
> +	    if (thr != NULL && thr->thread_fsm != NULL)
> +	      {
> +		should_notify_stop
> +		  = thread_fsm_should_notify_stop (thr->thread_fsm);
> +	      }
>  
> -	  if (should_notify_stop)
> -	    {
> -	      /* We may not find an inferior if this was a process exit.  */
> -	      if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
> -		proceeded = normal_stop ();
> -	    }
> +	    if (should_notify_stop)
> +	      {
> +		/* We may not find an inferior if this was a process exit.  */
> +		if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
> +		  proceeded = normal_stop ();
> +	      }
>  
> -	  if (!proceeded)
> -	    {
> -	      inferior_event_handler (INF_EXEC_COMPLETE, NULL);
> -	      cmd_done = 1;
> -	    }
> -	}
> -    }
> +	    if (!proceeded)
> +	      {
> +		inferior_event_handler (INF_EXEC_COMPLETE, NULL);
> +		cmd_done = 1;
> +	      }
> +	  }
> +      }
>  
> -  discard_cleanups (ts_old_chain);
> +    defer_delete_threads.release ();
> +    defer_bpstat_clear.release ();
>  
> -  /* No error, don't finish the thread states yet.  */
> -  finish_state.release ();
> +    /* No error, don't finish the thread states yet.  */
> +    finish_state.release ();
>  
> -  /* Revert thread and frame.  */
> -  do_cleanups (old_chain);
> +    /* This scope is used to ensure that readline callbacks are
> +       reinstalled here.  */
> +  }
>  
>    /* If a UI was in sync execution mode, and now isn't, restore its
>       prompt (a synchronous execution command has finished, and we're
> @@ -4284,14 +4275,6 @@ save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
>      }
>  }
>  
> -/* A cleanup that disables thread create/exit events.  */
> -
> -static void
> -disable_thread_events (void *arg)
> -{
> -  target_thread_events (0);
> -}
> -
>  /* See infrun.h.  */
>  
>  void
> @@ -4300,7 +4283,6 @@ stop_all_threads (void)
>    /* We may need multiple passes to discover all threads.  */
>    int pass;
>    int iterations = 0;
> -  struct cleanup *old_chain;
>  
>    gdb_assert (target_is_non_stop_p ());
>  
> @@ -4310,7 +4292,7 @@ stop_all_threads (void)
>    scoped_restore_current_thread restore_thread;
>  
>    target_thread_events (1);
> -  old_chain = make_cleanup (disable_thread_events, NULL);
> +  SCOPE_EXIT { target_thread_events (0); };
>  
>    /* Request threads to stop, and then wait for the stops.  Because
>       threads we already know about can spawn more threads while we're
> @@ -4495,8 +4477,6 @@ stop_all_threads (void)
>  	}
>      }
>  
> -  do_cleanups (old_chain);
> -
>    if (debug_infrun)
>      fprintf_unfiltered (gdb_stdlog, "infrun: stop_all_threads done\n");
>  }
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index c0d5f8dc66..45da9fa997 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -66,6 +66,7 @@
>  #include "objfiles.h"
>  #include "nat/linux-namespaces.h"
>  #include "fileio.h"
> +#include "common/scope-exit.h"
>  
>  #ifndef SPUFS_MAGIC
>  #define SPUFS_MAGIC 0x23c9b64e
> @@ -4223,22 +4224,10 @@ linux_nat_xfer_osdata (enum target_object object,
>      return TARGET_XFER_OK;
>  }
>  
> -static void
> -cleanup_target_stop (void *arg)
> -{
> -  ptid_t *ptid = (ptid_t *) arg;
> -
> -  gdb_assert (arg != NULL);
> -
> -  /* Unpause all */
> -  target_continue_no_signal (*ptid);
> -}
> -
>  std::vector<static_tracepoint_marker>
>  linux_nat_target::static_tracepoint_markers_by_strid (const char *strid)
>  {
>    char s[IPA_CMD_BUF_SIZE];
> -  struct cleanup *old_chain;
>    int pid = inferior_ptid.pid ();
>    std::vector<static_tracepoint_marker> markers;
>    const char *p = s;
> @@ -4253,7 +4242,8 @@ linux_nat_target::static_tracepoint_markers_by_strid (const char *strid)
>  
>    agent_run_command (pid, s, strlen (s) + 1);
>  
> -  old_chain = make_cleanup (cleanup_target_stop, &ptid);
> +  /* Unpause all.  */
> +  SCOPE_EXIT { target_continue_no_signal (ptid); };
>  
>    while (*p++ == 'm')
>      {
> @@ -4272,8 +4262,6 @@ linux_nat_target::static_tracepoint_markers_by_strid (const char *strid)
>        p = s;
>      }
>  
> -  do_cleanups (old_chain);
> -
>    return markers;
>  }
>  
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index c51ef771be..bbaca73ff8 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -219,37 +219,6 @@ reg_buffer::arch () const
>    return m_descr->gdbarch;
>  }
>  
> -/* Cleanup class for invalidating a register.  */
> -
> -class regcache_invalidator
> -{
> -public:
> -
> -  regcache_invalidator (struct regcache *regcache, int regnum)
> -    : m_regcache (regcache),
> -      m_regnum (regnum)
> -  {
> -  }
> -
> -  ~regcache_invalidator ()
> -  {
> -    if (m_regcache != nullptr)
> -      m_regcache->invalidate (m_regnum);
> -  }
> -
> -  DISABLE_COPY_AND_ASSIGN (regcache_invalidator);
> -
> -  void release ()
> -  {
> -    m_regcache = nullptr;
> -  }
> -
> -private:
> -
> -  struct regcache *m_regcache;
> -  int m_regnum;
> -};
> -
>  /* Return  a pointer to register REGNUM's buffer cache.  */
>  
>  gdb_byte *
> @@ -769,7 +738,8 @@ regcache::raw_write (int regnum, const gdb_byte *buf)
>  
>    /* Invalidate the register after it is written, in case of a
>       failure.  */
> -  regcache_invalidator invalidator (this, regnum);
> +  auto invalidator
> +    = make_scope_exit ([&] { this->invalidate (regnum); });
>  
>    target_store_registers (this, regnum);
>  
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 04197120db..a9f03790b6 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -59,6 +59,7 @@
>  #include "common/byte-vector.h"
>  #include "selftest.h"
>  #include "cli/cli-style.h"
> +#include "common/scope-exit.h"
>  
>  #include <sys/types.h>
>  #include <fcntl.h>
> @@ -79,8 +80,6 @@ void (*deprecated_show_load_progress) (const char *section,
>  void (*deprecated_pre_add_symbol_hook) (const char *);
>  void (*deprecated_post_add_symbol_hook) (void);
>  
> -static void clear_symtab_users_cleanup (void *ignore);
> -
>  /* Global variables owned by this file.  */
>  int readnow_symbol_files;	/* Read full symbols immediately.  */
>  int readnever_symbol_files;	/* Never read full symbols.  */
> @@ -893,6 +892,9 @@ init_entry_point_info (struct objfile *objfile)
>      }
>  }
>  
> +using clear_symtab_users_cleanup
> +  = FORWARD_SCOPE_EXIT (clear_symtab_users);
> +
>  /* Process a symbol file, as either the main file or as a dynamically
>     loaded file.
>  
> @@ -923,7 +925,6 @@ syms_from_objfile_1 (struct objfile *objfile,
>  		     symfile_add_flags add_flags)
>  {
>    section_addr_info local_addr;
> -  struct cleanup *old_chain;
>    const int mainline = add_flags & SYMFILE_MAINLINE;
>  
>    objfile_set_sym_fns (objfile, find_sym_fns (objfile->obfd));
> @@ -945,7 +946,9 @@ syms_from_objfile_1 (struct objfile *objfile,
>  
>    /* Make sure that partially constructed symbol tables will be cleaned up
>       if an error occurs during symbol reading.  */
> -  old_chain = make_cleanup (null_cleanup, NULL);
> +
> +  gdb::optional<clear_symtab_users_cleanup> defer_clear_users;
> +
>    std::unique_ptr<struct objfile> objfile_holder (objfile);
>  
>    /* If ADDRS is NULL, put together a dummy address list.
> @@ -958,7 +961,7 @@ syms_from_objfile_1 (struct objfile *objfile,
>      {
>        /* We will modify the main symbol table, make sure that all its users
>           will be cleaned up if an error occurs during symbol reading.  */
> -      make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
> +      defer_clear_users.emplace ((symfile_add_flag) 0);
>  
>        /* Since no error yet, throw away the old symbol table.  */
>  
> @@ -999,7 +1002,8 @@ syms_from_objfile_1 (struct objfile *objfile,
>    /* Discard cleanups as symbol reading was successful.  */
>  
>    objfile_holder.release ();
> -  discard_cleanups (old_chain);
> +  if (defer_clear_users)
> +    defer_clear_users->release ();
>  }
>  
>  /* Same as syms_from_objfile_1, but also initializes the objfile
> @@ -2441,7 +2445,6 @@ reread_symbols (void)
>        new_modtime = new_statbuf.st_mtime;
>        if (new_modtime != objfile->mtime)
>  	{
> -	  struct cleanup *old_cleanups;
>  	  struct section_offsets *offsets;
>  	  int num_offsets;
>  
> @@ -2461,7 +2464,7 @@ reread_symbols (void)
>  	  std::unique_ptr<struct objfile> objfile_holder (objfile);
>  
>  	  /* We need to do this whenever any symbols go away.  */
> -	  old_cleanups = make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
> +	  clear_symtab_users_cleanup defer_clear_users (0);
>  
>  	  if (exec_bfd != NULL
>  	      && filename_cmp (bfd_get_filename (objfile->obfd),
> @@ -2615,7 +2618,7 @@ reread_symbols (void)
>  
>  	  /* Discard cleanups as symbol reading was successful.  */
>  	  objfile_holder.release ();
> -	  discard_cleanups (old_cleanups);
> +	  defer_clear_users.release ();
>  
>  	  /* If the mtime has changed between the time we set new_modtime
>  	     and now, we *want* this to be out of date, so don't call stat
> @@ -2892,12 +2895,6 @@ clear_symtab_users (symfile_add_flags add_flags)
>    if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
>      breakpoint_re_set ();
>  }
> -
> -static void
> -clear_symtab_users_cleanup (void *ignore)
> -{
> -  clear_symtab_users (0);
> -}
>  
>  /* OVERLAYS:
>     The following code implements an abstraction for debugging overlay sections.
> diff --git a/gdb/top.c b/gdb/top.c
> index 900e78aaec..cf6a6abc7d 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -52,6 +52,7 @@
>  #include "frame.h"
>  #include "buffer.h"
>  #include "gdb_select.h"
> +#include "common/scope-exit.h"
>  
>  /* readline include files.  */
>  #include "readline/readline.h"
> @@ -539,12 +540,11 @@ set_repeat_arguments (const char *args)
>  void
>  execute_command (const char *p, int from_tty)
>  {
> -  struct cleanup *cleanup_if_error;
>    struct cmd_list_element *c;
>    const char *line;
>    const char *cmd_start = p;
>  
> -  cleanup_if_error = make_bpstat_clear_actions_cleanup ();
> +  auto cleanup_if_error = make_scope_exit (bpstat_clear_actions);
>    scoped_value_mark cleanup = prepare_execute_command ();
>  
>    /* Force cleanup of any alloca areas if using C alloca instead of
> @@ -554,7 +554,7 @@ execute_command (const char *p, int from_tty)
>    /* This can happen when command_line_input hits end of file.  */
>    if (p == NULL)
>      {
> -      discard_cleanups (cleanup_if_error);
> +      cleanup_if_error.release ();
>        return;
>      }
>  
> @@ -649,7 +649,7 @@ execute_command (const char *p, int from_tty)
>    if (has_stack_frames () && inferior_thread ()->state != THREAD_RUNNING)
>      check_frame_language_change ();
>  
> -  discard_cleanups (cleanup_if_error);
> +  cleanup_if_error.release ();
>  }
>  
>  /* Run execute_command for P and FROM_TTY.  Capture its output into the
> diff --git a/gdb/ui-out.h b/gdb/ui-out.h
> index 5f4eea5491..8d183060b5 100644
> --- a/gdb/ui-out.h
> +++ b/gdb/ui-out.h
> @@ -195,11 +195,9 @@ 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.  */
> +/* Start a new tuple or list on construction, and end it on
> +   destruction.  Normally this is used via the typedefs
> +   ui_out_emit_tuple and ui_out_emit_list.  */
>  template<ui_out_type Type>
>  class ui_out_emit_type
>  {
> diff --git a/gdb/utils.c b/gdb/utils.c
> index ed8d60fa7b..4af75e3480 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -3057,23 +3057,6 @@ parse_pid_to_attach (const char *args)
>    return pid;
>  }
>  
> -/* Helper for make_bpstat_clear_actions_cleanup.  */
> -
> -static void
> -do_bpstat_clear_actions_cleanup (void *unused)
> -{
> -  bpstat_clear_actions ();
> -}
> -
> -/* Call bpstat_clear_actions for the case an exception is throw.  You should
> -   discard_cleanups if no exception is caught.  */
> -
> -struct cleanup *
> -make_bpstat_clear_actions_cleanup (void)
> -{
> -  return make_cleanup (do_bpstat_clear_actions_cleanup, NULL);
> -}
> -
>  /* Substitute all occurences of string FROM by string TO in *STRINGP.  *STRINGP
>     must come from xrealloc-compatible allocator and it may be updated.  FROM
>     needs to be delimited by IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be
> diff --git a/gdb/utils.h b/gdb/utils.h
> index f2fe1da832..896feb973c 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -286,7 +286,6 @@ private:
>    int m_save_batch_flag;
>  };
>  
> -extern struct cleanup *make_bpstat_clear_actions_cleanup (void);
>  
>  /* Path utilities.  */
>  
> -- 
> 2.14.4
> 



More information about the Gdb-patches mailing list