Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: Pedro Alves <palves@redhat.com>
Cc: Tom Tromey <tom@tromey.com>,
	 Andrew Burgess <andrew.burgess@embecosm.com>,
	 gdb-patches@sourceware.org
Subject: Re: [PATCH 00/12] remove some cleanups using a cleanup function
Date: Thu, 17 Jan 2019 21:39:00 -0000	[thread overview]
Message-ID: <87ef9bm0a8.fsf@tromey.com> (raw)
In-Reply-To: <f5c1d5d3-7806-6010-1f37-6d0af9555920@redhat.com> (Pedro Alves's	message of "Wed, 16 Jan 2019 23:10:23 +0000")

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> See patch below.  Since cleanup_function turned into scope_exit,
Pedro> this new class became forward_scope_exit, because it's a "forwarding"
Pedro> version of scope_exit?  I'm really not that sure about that name...
Pedro> wrap_scope_exit came to mind too.  Or maybe call it cleanup_function?

The name seems reasonable enough to me.

Pedro> Ideally we'd find a way to implement SCOPE_FAILURE/SCOPE_SUCCESS
Pedro> in C++11 instead...

Maybe we should start a C++17 meta-bug to collect future changes.

Pedro> I think this is the first time I had found a use for a function-try-block,
Pedro> see scope_exit ctor...

TIL!

Pedro> Here is patch with everything together.  WDYT?

I think it all looks good.  One nit and one question...

Pedro> +/* A forward_scope_exit is like scope_exit, but instead of giving it a
Pedro> +   callable, you instead specialize it for a given cleanup function,
Pedro> +   and the generated class automatically has a constructor with the

I think it would make sense to explain the origin of the "forward" name
somewhere in this comment.

Pedro> +#define SCOPE_EXIT \
Pedro> +  auto CONCAT(scope_exit_, __LINE__) = ::detail::scope_exit_lhs () + [&] ()
[...]
Pedro> +  auto invalidator
Pedro> +    = make_scope_exit ([&] { this->invalidate (regnum); });
 
In the current spots it doesn't matter, but I tend to think it's better
to capture by value rather than by reference.  The local being captured
might well be reused in the function.

On the other hand, this would be a difference from the spec.

Tom


  reply	other threads:[~2019-01-17 21:39 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-12 11:50 Andrew Burgess
2019-01-09  3:34 ` Tom Tromey
2019-01-09  3:34   ` [PATCH 01/12] Remove delete_longjmp_breakpoint_cleanup Tom Tromey
2019-01-09  3:34   ` [PATCH 11/12] Update cleanup comment in ui-out.h Tom Tromey
2019-01-09  3:34   ` [PATCH 03/12] Remove make_bpstat_clear_actions_cleanup Tom Tromey
2019-01-09  3:34   ` [PATCH 05/12] Remove cleanup from linux-nat.c Tom Tromey
2019-01-09  3:34   ` [PATCH 07/12] Remove delete_just_stopped_threads_infrun_breakpoints_cleanup Tom Tromey
2019-01-09  3:34   ` [PATCH 04/12] Remove cleanup_delete_std_terminate_breakpoint Tom Tromey
2019-01-09  3:34   ` [PATCH 08/12] Remove cleanup from stop_all_threads Tom Tromey
2019-01-09  3:34   ` [PATCH 09/12] Remove remaining cleanup from fetch_inferior_event Tom Tromey
2019-01-09  3:34   ` [PATCH 12/12] Use cleanup_function in regcache.c Tom Tromey
2019-01-09  3:34   ` [PATCH 06/12] Remove clear_symtab_users_cleanup Tom Tromey
2019-01-09  3:34   ` [PATCH 10/12] Update an obsolete cleanup comment Tom Tromey
2019-01-09  3:36   ` [PATCH 02/12] Remove remaining cleanup from breakpoint.c Tom Tromey
2019-01-11  6:56   ` [PATCH 00/12] remove some cleanups using a cleanup function Joel Brobecker
2019-01-12 11:50   ` [PATCH 1/4] gdb: Remove delete_longjmp_breakpoint_cleanup Andrew Burgess
2019-01-12 11:50   ` [PATCH 3/4] gdb: Remove make_bpstat_clear_actions_cleanup Andrew Burgess
2019-01-12 11:50   ` [PATCH 4/4] gdb/testsuite: Don't allow paths to appear in test name Andrew Burgess
2019-01-12 11:50   ` [PATCH 2/4] gdb: Remove remaining cleanup from breakpoint.c Andrew Burgess
2019-01-12 15:54 ` [PATCH 00/12] remove some cleanups using a cleanup function Tom Tromey
2019-01-12 22:41   ` Tom Tromey
2019-01-14 11:06     ` Andrew Burgess
2019-01-14 15:39       ` Pedro Alves
2019-01-14 20:37 ` Pedro Alves
2019-01-15  9:42   ` Andrew Burgess
     [not found]     ` <87ef9dttfl.fsf@tromey.com>
2019-01-15 23:43       ` Pedro Alves
2019-01-16 11:19         ` Andrew Burgess
2019-01-16 23:10         ` Pedro Alves
2019-01-17 21:39           ` Tom Tromey [this message]
2019-01-21 20:12             ` Pedro Alves

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=87ef9bm0a8.fsf@tromey.com \
    --to=tom@tromey.com \
    --cc=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox