[RFA,09/22] Remove make_cleanup_restore_current_ui

  This removes make_cleanup_restore_current_ui by converting the last
use.  The last use was in a few functions used to iterate over all
UIs.  This patch replaces these functions with a class, and arranges
for the class destructor to do the needed cleanup.

2016-09-26  Tom Tromey  <tom@tromey.com>

	* tui/tui-interp.c (tui_on_normal_stop, tui_on_signal_received)
	(tui_on_end_stepping_range, tui_on_signal_exited, tui_on_exited)
	(tui_on_no_history): Update.
	* top.h (switch_thru_all_uis): New class.
	(SWITCH_THRU_ALL_UIS): Rewrite.
	(make_cleanup_restore_current_ui, switch_thru_all_uis_init)
	(switch_thru_all_uis_cond, switch_thru_all_uis_next): Don't
	declare.
	* mi/mi-interp.c (mi_new_thread, mi_thread_exit)
	(mi_record_changed, mi_inferior_added, mi_inferior_appeared)
	(mi_inferior_exit, mi_inferior_removed, mi_on_signal_received)
	(mi_on_end_stepping_range, mi_on_signal_exited, mi_on_exited)
	(mi_on_no_history, mi_on_normal_stop, mi_traceframe_changed)
	(mi_tsv_created, mi_tsv_deleted, mi_tsv_modified)
	(mi_breakpoint_created, mi_breakpoint_deleted)
	(mi_breakpoint_modified, mi_output_running_pid, mi_on_resume)
	(mi_solib_loaded, mi_solib_unloaded, mi_command_param_changed)
	(mi_memory_changed): Update.
	* infrun.c (all_uis_check_sync_execution_done)
	(all_uis_on_sync_execution_starting, normal_stop): Update.
	* event-top.c (restore_ui_cleanup)
	(make_cleanup_restore_current_ui, switch_thru_all_uis_init)
	(switch_thru_all_uis_cond, switch_thru_all_uis_next): Remove.
	* cli/cli-interp.c (cli_on_normal_stop, cli_on_signal_received)
	(cli_on_end_stepping_range, cli_on_signal_exited, cli_on_exited)
	(cli_on_no_history): Update.
	* breakpoint.c (watchpoint_check): Update.
---
 gdb/ChangeLog        |  30 +++++++++++++++
 gdb/breakpoint.c     |   8 +---
 gdb/cli/cli-interp.c |  24 +++---------
 gdb/event-top.c      |  50 -------------------------
 gdb/infrun.c         |  15 +++-----
 gdb/mi/mi-interp.c   | 101 +++++++++++++--------------------------------------
 gdb/top.h            |  56 +++++++++++++++++++---------
 gdb/tui/tui-interp.c |  24 +++---------
 8 files changed, 114 insertions(+), 194 deletions(-)
  

> +  ~switch_thru_all_uis ()
> +  {
> +  }

So, you don't need to do anything  special here because of
scoped_restore.  So why not just use the compilers definition of this?

Trev
  

>>>>> "Trevor" == Trevor Saunders <tbsaunde@tbsaunde.org> writes:

>> +  ~switch_thru_all_uis ()
>> +  {
>> +  }

Trev> So, you don't need to do anything  special here because of
Trev> scoped_restore.  So why not just use the compilers definition of this?

I made this change.

Tom
  

On 2016-09-27 00:08, Tom Tromey wrote:
> --- a/gdb/top.h
> +++ b/gdb/top.h
> @@ -155,27 +155,52 @@ extern struct ui *current_ui;
>  /* The list of all UIs.  */
>  extern struct ui *ui_list;
> 
> -/* State for SWITCH_THRU_ALL_UIS.  Declared here because it is meant
> -   to be created on the stack, but should be treated as opaque.  */
> -struct switch_thru_all_uis
> +/* State for SWITCH_THRU_ALL_UIS.  */
> +class switch_thru_all_uis
>  {
> +public:
> +
> +  switch_thru_all_uis () : iter (nullptr), save_ui (&current_ui)

We are targetting C++98, aren't we?  Or is it C++03?  Either way, 
nullptr appeared in C++11, so I guess we can't use it.

The fact that your compiler (and mine) did not catch this begs the 
question, should we have one of -std=c++98/gnu++98/c++03/gnu++03 in our 
compilation flags, instead of relying of the compiler's default mode?

> +  {
> +    iter = ui_list;
> +  }
> +
> +  ~switch_thru_all_uis ()
> +  {
> +  }
> +
> +  // If done iterating, return true; otherwise return false.
> +  bool done () const
> +  {
> +    return iter == nullptr;
> +  }
> +
> +  // Move to the next UI, setting current_ui if iteration is not yet
> +  // complete.
> +  void next ()
> +  {
> +    iter = iter->next;
> +    if (iter != nullptr)
> +      current_ui = iter;
> +  }
> +
> + private:
> +
> +  // No need for these.  They are intentionally not defined anywhere.
> +  switch_thru_all_uis &operator= (const switch_thru_all_uis &);
> +  switch_thru_all_uis (const switch_thru_all_uis &);
> +
> +  // Used to iterate through the UIs.
>    struct ui *iter;
> -  struct cleanup *old_chain;
> -};
> 
> -/* Functions to drive SWITCH_THRU_ALL_UIS.  Though declared here by
> -   necessity, these functions should not be used other than via the
> -   SWITCH_THRU_ALL_UIS macro defined below.  */
> -extern void switch_thru_all_uis_init (struct switch_thru_all_uis 
> *state);
> -extern int switch_thru_all_uis_cond (struct switch_thru_all_uis 
> *state);
> -extern void switch_thru_all_uis_next (struct switch_thru_all_uis 
> *state);
> +  // Save and restore current_ui.
> +  scoped_restore<struct ui *> save_ui;
> +};
> 
>    /* Traverse through all UI, and switch the current UI to the one
>       being iterated.  */
>  #define SWITCH_THRU_ALL_UIS(STATE)		\

You can remove STATE here.  I am surprised the preprocessor doesn't care 
about the missing argument in macro references.
  

On 09/27/2016 05:08 AM, Tom Tromey wrote:
> +class switch_thru_all_uis
>  {
> +public:
> +
> +  switch_thru_all_uis () : iter (nullptr), save_ui (&current_ui)
> +  {
> +    iter = ui_list;

This initializes "iter" twice.

Thanks,
Pedro Alves
  

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

>> +class switch_thru_all_uis
>> {
>> +public:
>> +
>> +  switch_thru_all_uis () : iter (nullptr), save_ui (&current_ui)
>> +  {
>> +    iter = ui_list;

Pedro> This initializes "iter" twice.

Thanks.  Apparently I already found that one and fixed it on my branch,
because now I see:

+  switch_thru_all_uis () : m_iter (ui_list), m_save_ui (&current_ui)
+  {
+  }
+

Tom
  

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> We are targetting C++98, aren't we?  Or is it C++03?  Either way,
Simon> nullptr appeared in C++11, so I guess we can't use it.

Yep.  Fixed.

>> #define SWITCH_THRU_ALL_UIS(STATE)		\

Simon> You can remove STATE here.  I am surprised the preprocessor doesn't
Simon> care about the missing argument in macro references.

Maybe that's a GNU extension.  I forget.  But anyway I've fixed it.

Tom
  

On 2016-09-27 00:08, Tom Tromey wrote:
> +/* State for SWITCH_THRU_ALL_UIS.  */
> +class switch_thru_all_uis
>  {
> +public:
> +
> +  switch_thru_all_uis () : iter (nullptr), save_ui (&current_ui)
> +  {
> +    iter = ui_list;
> +  }

Hi Tom,

I just found there is a little bug here.  current_ui is not set for the 
first iteration, so it runs with whatever was in there before.  If you 
look at the previous implementation, current_ui is assigned in the _cond 
function, which is executed before the first iteration.  In your 
version, it is assigned in the next() method, which isn't.

I noticed this because it fails gdb.mi/user-selected-context-sync.exp.

Simon
  

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> +  switch_thru_all_uis () : iter (nullptr), save_ui (&current_ui)
>> +  {
>> +    iter = ui_list;
>> +  }

Simon> I just found there is a little bug here.
[...]

Thanks.  I'll fix it before re-posting the series.
Not sure if I should do that before the other patches are reviewed though.

Tom
  

On 10/09/2016 04:10 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
>>> +  switch_thru_all_uis () : iter (nullptr), save_ui (&current_ui)
>>> +  {
>>> +    iter = ui_list;
>>> +  }
> 
> Simon> I just found there is a little bug here.
> [...]
> 
> Thanks.  I'll fix it before re-posting the series.
> Not sure if I should do that before the other patches are reviewed though.

I _think_ they've all been reviewed now.  Please push in
the ones that are independent and OK already, and repost the
others.

Thanks,
Pedro Alves
  

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

Pedro> I _think_ they've all been reviewed now.  Please push in
Pedro> the ones that are independent and OK already, and repost the
Pedro> others.

Ok.  I think after all is said and done, there are 5 that can go in and
the rest need something -- either final review on scoped_restore or
unique_ptr.

BTW, I'm wondering if you want scoped_restore in its own header.  It
didn't occur to me while writing it, but now it seems like a good idea.

Tom
  

On 10/12/2016 11:43 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I _think_ they've all been reviewed now.  Please push in
> Pedro> the ones that are independent and OK already, and repost the
> Pedro> others.
> 
> Ok.  I think after all is said and done, there are 5 that can go in and
> the rest need something -- either final review on scoped_restore or
> unique_ptr.

Yeah, sounds right.  I was trying get the gdb::unique_ptr in, in order
to unblock the cases I suggested you use unique_ptr, but I'm a bit
confused on what to do about it now...  I _think_ people are generally
OK with it.  There was some opposition, but I'm not sure anymore
whether it still exists.  C++11 is now on the table, but maybe a
staged approach (enable C++11 while supporting C++03 too for a while,
to catch issues) would make sense anyway.
But I'd really like to move forward with deciding on _some_
smart pointer to use, in order to unblock further conversion.
I'll re-review tomorrow with a fresher head (and give people time to
comment on actual implementation details (or just say they mean to),
if they want...)

> BTW, I'm wondering if you want scoped_restore in its own header.  It
> didn't occur to me while writing it, but now it seems like a good idea.

It does sound like a good idea.

Thanks,
Pedro Alves
  

> Cc: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 13 Oct 2016 02:28:44 +0100
> 
> Yeah, sounds right.  I was trying get the gdb::unique_ptr in, in order
> to unblock the cases I suggested you use unique_ptr, but I'm a bit
> confused on what to do about it now...  I _think_ people are generally
> OK with it.  There was some opposition, but I'm not sure anymore
> whether it still exists.  C++11 is now on the table, but maybe a
> staged approach (enable C++11 while supporting C++03 too for a while,
> to catch issues) would make sense anyway.

I still think we shouldn't have workarounds for versions of the
standard older than what we want to support.  IOW, if we decide to use
C++11, we should _require_ a compiler that supports it, and error out
if the configure test(s) regarding that fail.

Supporting more than one standard means we will have rarely used code
that is almost never tested, and that means maintenance burden which
threaten to undo at least some part of the benefits you aspire to
achieve by moving to C++ in general and to a new enough C++ standard
in particular.
  

On 10/13/2016 07:11 AM, Eli Zaretskii wrote:
>> Cc: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Thu, 13 Oct 2016 02:28:44 +0100
>>
>> Yeah, sounds right.  I was trying get the gdb::unique_ptr in, in order
>> to unblock the cases I suggested you use unique_ptr, but I'm a bit
>> confused on what to do about it now...  I _think_ people are generally
>> OK with it.  There was some opposition, but I'm not sure anymore
>> whether it still exists.  C++11 is now on the table, but maybe a
>> staged approach (enable C++11 while supporting C++03 too for a while,
>> to catch issues) would make sense anyway.
> 
> I still think we shouldn't have workarounds for versions of the
> standard older than what we want to support.

That is not what the patch does...  

#1 - Starting from the position that gdb wants to build with C++03
     compilers.

#2 - The patch adds a new smart pointer type.  In C++03, you're pretty
     much guaranteed to need to do that.  The one the standard gives you
     is too limited.

#3 - (owning) smart pointers all look the same for most client code - they're
     meant to look like raw pointers afterall - i.e., ptr->foo, and *ptr
     works as expected, transparently.

#4 - where owning smart pointer APIs tend to differ is in code around pointee
     ownership.

     I chose to model the API on std::unique_ptr (C++11).
     There are advantages.

#5 - We could stop here.  Done.

#6 - But I went a step further, and made it so that if we have C++11
     handy, then we can make use of the compiler to catch bugs at
     compile time, all while keeping the nice API.

So again:

> I still think we shouldn't have workarounds for versions of the
> standard older than what we want to support.

there's no workaround for versions older than what we want to support
at all.  There's the code for the version that we had decided earlier
that we wanted to support, plus extra API enforcement when built
with a compiler that supports a version _newer_ than what we want
to support.


I'll give you more examples of why I think forbidding C++11 completely
is short sighted.  C++11 has a nice feature where you can tell the compiler
that some class is "final" (not possible to to inherit from that class).
This allows the compiler to optimize the code better.

Another nice C++11 feature is that it's possible to mark class methods
as overriding a virtual method in the base class.  This is quite handy,
because with C++03, if you change the method's prototype in base class,
you have to remember to update all users.  And without the "override" marker,
the compiler won't help you find them at all.  The program builds, and then
fails at run time.  Maybe only after release do you notice it.
With the override marker, the compiler now errors out pointing at all
the places you need to fix.  Bug found at compiler time.  Wonderful.

So if we have a C++11 compiler handy, why not make use of these
features to help with development?  I don't see how it makes
sense to not take advantage of compiler help if available.

Here's GCC conditionally using these features if available as well:

  https://gcc.gnu.org/ml/jit/2016-q2/msg00000.html

And GCC still supports building with a C++03 compiler.  Just
like GDB.  Tons of projects out there do the same thing.

> IOW, if we decide to use
> C++11, we should _require_ a compiler that supports it, and error out
> if the configure test(s) regarding that fail.
> 

So:

#1.1 - for people with new compilers, we'd go through C++11 std::unique_ptr
       all the time.

#1.2 - people with older compilers would be out of luck.  tough.  get newer
       compiler first.

Vs:

#2.1 - for people with new compilers, we'd go through std::unique_ptr all
       the time.

#2.2 - for people with older compilers we go through the fallback
       implementation.  gdb still works.

(I wasn't planning on starting the discussion on whether 1.2 is
reasonable nowadays.  That happened because someone else brought it up.)


So for people with newer compilers, the result is the same.
(#1.1 == #1.2).  While I'm proposing a solution that gives us all the
benefits (#2.1) while keeping support for older compilers
for a little while longer (#2.2).

> Supporting more than one standard means we will have rarely used code
> that is almost never tested, and that means maintenance burden 

That same argument could be applied to gnulib.  Or the many HAVE_FOO's
in the code base.  We rely on gnulib's reimplementation of the whole
printf family of functions if the compiler doesn't support it properly
for example!

So what if the fallback code is only used on older compilers?
If you're willing to go #1.1/#2.1 above, then what do you lose?

The sane way to make sure the fallback code still works is
via autotesting with the buildbot.

> that means maintenance burden 

For whom?  _I'm_ willing to maintain this code.  It's not rocket
science code.  And then users of the smart pointer don't really
have to care.  Whether C++11 or C++03, you write the same code.
You may miss a gdb::move call with C++03.  No big deal.  The fix
is just to add a gdb::move call.

Several others have said they think this is a good idea.  I've even
ran this idea through the libstdc++ maintainer in person at the Cauldron.
Trevor said he wants to do this in gcc as well.

If redirecting to C++11 std::unique_ptr turns out to cause trouble, or
I'm hit by a bus, then it's trivial to remove that redirection.  All it
takes is remove a handful of lines of code guarded with
#if __cplusplus >= 201103 to always go through the fallback implementation.

So there's no "point of no return here".

> which
> threaten to undo at least some part of the benefits you aspire to
> achieve by moving to C++ in general and to a new enough C++ standard
> in particular.

I don't understand what this is trying to say.

I propose to move forward with my plan, and if it does cause
trouble, then we drop the gdb::unique_ptr -> std::unique_ptr mapping.

It may happen, I don't know.  We will know if it will indeed cause
trouble in practice if we try.

That will allow moving forward with cleanup elimination and more.

And _in_ parallel, as a separate discussion, we can discuss dropping
support for C++03 and going C++11 fully.  I think we could pull it
off, but I wouldn't want to do it in a rush, and without an easy
fallback plan available.  The patch I sent yesterday to make use
of C++11 with gcc >= 4.8 would be the first step.  Then if that doesn't
cause problems, we can tell the autoconf macro to require C++11
with a single line change.  If _requiring_ C++11 causes significant
trouble on people, and we want to go back to supporting older compilers,
then it's easy to revert that, and go back to the unique_ptr C++03
fallback implementation.  This is header-only template code, so we
really can't tell if this will cause trouble for people without
adding actual _uses_ of the smart pointer.

> IOW, if we decide to use
> C++11, we should _require_ a compiler that supports it, and error out
> if the configure test(s) regarding that fail.

In the patch I sent yesterday to teach gdb's configure about C++11
in gcc >= 4.8, if we change the macro invocation to make C++11
mandatory, then if configure doesn't find a C++11 compiler,
it errors out with a nice error message.

And no, I'm not planning on getting hit by a bus.  :-)

Thanks,
Pedro Alves
  

> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 13 Oct 2016 11:16:26 +0100
> 
> On 10/13/2016 07:11 AM, Eli Zaretskii wrote:
> >> Cc: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
> >> From: Pedro Alves <palves@redhat.com>
> >> Date: Thu, 13 Oct 2016 02:28:44 +0100
> >>
> >> Yeah, sounds right.  I was trying get the gdb::unique_ptr in, in order
> >> to unblock the cases I suggested you use unique_ptr, but I'm a bit
> >> confused on what to do about it now...  I _think_ people are generally
> >> OK with it.  There was some opposition, but I'm not sure anymore
> >> whether it still exists.  C++11 is now on the table, but maybe a
> >> staged approach (enable C++11 while supporting C++03 too for a while,
> >> to catch issues) would make sense anyway.
> > 
> > I still think we shouldn't have workarounds for versions of the
> > standard older than what we want to support.
> 
> That is not what the patch does...  

I wasn't referring to the patch, I was referring to the quoted part of
your message, specifically to this:

> >> C++11 is now on the table, but maybe a staged approach (enable
> >> C++11 while supporting C++03 too for a while, to catch issues)
> >> would make sense anyway.

> So again:
> 
> > I still think we shouldn't have workarounds for versions of the
> > standard older than what we want to support.
> 
> there's no workaround for versions older than what we want to support
> at all.

Since I'm still unclear what is the oldest C++ standard we want to
support, I cannot decide whether I agree or disagree.  Is the version
of the C++ standard we support C++03 or C++11?  My comment assumed
that it was C++11.

> I'll give you more examples of why I think forbidding C++11 completely
> is short sighted.

Thanks, but that kind of arguments completely misses the point.  You
don't need to convince me that a later standard is better than the
previous ones.  The issue at hand is whether we should write code that
targets more than a single language standard, where these standards
aren't compatible.  I think we shouldn't.

> > Supporting more than one standard means we will have rarely used code
> > that is almost never tested, and that means maintenance burden 
> 
> That same argument could be applied to gnulib.  Or the many HAVE_FOO's
> in the code base.  We rely on gnulib's reimplementation of the whole
> printf family of functions if the compiler doesn't support it properly
> for example!

I did apply it to gnulib at the time, but was voted down.

Do you seriously consider the gnulib solution a good one?  I don't.
The code obfuscation is very significant.  Of course, as long as you
don't look into the gnulib headers, you don't care, but in this case
you propose to have all those shims in our sources, in plain sight.

> > that means maintenance burden 
> 
> For whom?  _I'm_ willing to maintain this code.

Are you saying that no one else will be allowed to modify the code
which has 2 branches, one for C++03, the other for C++11?  I'm betting
you aren't saying that, and so this is a burden for all of us, not
just for you.  And that is even before we consider the "bus issue".

> It's not rocket science code.

Today it isn't.  I'm not talking about this particular patch, I'm
talking about this policy in general.  As do you, I suppose: you
aren't just asking for agreement about using C++11 features in this
single case, right?

> Several others have said they think this is a good idea.  I've even
> ran this idea through the libstdc++ maintainer in person at the Cauldron.
> Trevor said he wants to do this in gcc as well.

I just wanted to voice my opposition, that's all.  I don't have to
give up just because a few others think otherwise.  Right?

> If redirecting to C++11 std::unique_ptr turns out to cause trouble, or
> I'm hit by a bus, then it's trivial to remove that redirection.  All it
> takes is remove a handful of lines of code guarded with
> #if __cplusplus >= 201103 to always go through the fallback implementation.

Once again, it's not just this single patch that bothers me.  Once we
have enough of these #if's, removing them is not necessarily a trivial
matter, especially when most of the builds, perhaps even all of them,
have been using the C++11 code path all the time, and the other one
has simply bitrotted.

> > which
> > threaten to undo at least some part of the benefits you aspire to
> > achieve by moving to C++ in general and to a new enough C++ standard
> > in particular.
> 
> I don't understand what this is trying to say.

The move to C++ was supposed to make the maintenance easier, right?
I'm saying that targeting more than a single language standard makes
maintenance harder, so it works against the purpose of the move.

> And no, I'm not planning on getting hit by a bus.  :-)

Who is?
  

On 10/13/2016 02:52 PM, Eli Zaretskii wrote:

> Since I'm still unclear what is the oldest C++ standard we want to
> support, I cannot decide whether I agree or disagree.  Is the version
> of the C++ standard we support C++03 or C++11?  My comment assumed
> that it was C++11.

It was decided in 2014 that we'd target C++03.  We have not decided
yet to _require_ C++11.

> 
>> I'll give you more examples of why I think forbidding C++11 completely
>> is short sighted.
> 
> Thanks, but that kind of arguments completely misses the point.  You
> don't need to convince me that a later standard is better than the
> previous ones.  The issue at hand is whether we should write code that
> targets more than a single language standard, where these standards
> aren't compatible.  I think we shouldn't.

They are not incompatible.  C++03 code compiles with a C++11 compiler
just fine.

> 
>>> Supporting more than one standard means we will have rarely used code
>>> that is almost never tested, and that means maintenance burden 
>>
>> That same argument could be applied to gnulib.  Or the many HAVE_FOO's
>> in the code base.  We rely on gnulib's reimplementation of the whole
>> printf family of functions if the compiler doesn't support it properly
>> for example!
> 
> I did apply it to gnulib at the time, but was voted down.
> 
> Do you seriously consider the gnulib solution a good one?  I don't.

Yes, I do.

> The code obfuscation is very significant.  Of course, as long as you
> don't look into the gnulib headers, you don't care, but in this case
> you propose to have all those shims in our sources, in plain sight.

Yes, "all those shims".  One file.  And then cleaner code in thousands
of uses of the shim throughout the codebase.  None of that
shim-using code needs to know what version of the standard is
being targeted.  Take a look at patch #3 from the gdb::unique_ptr
series, and see for yourself.

So I'll take that tradeoff, yes.

> 
>>> that means maintenance burden 
>>
>> For whom?  _I'm_ willing to maintain this code.
> 
> Are you saying that no one else will be allowed to modify the code
> which has 2 branches, one for C++03, the other for C++11?  I'm betting
> you aren't saying that, 

Of course not.

> and so this is a burden for all of us, not just for you.  

You're automatically assuming it's a burden.  I believe that's
false.

> 
>> It's not rocket science code.
> 
> Today it isn't.  I'm not talking about this particular patch, I'm
> talking about this policy in general.  As do you, I suppose: you
> aren't just asking for agreement about using C++11 features in this
> single case, right?

I'm talking about adding a small utility that is going to be used 
extensively throughout the codebase.  A smart pointer is a central widget.
That mine was modeled on a C++11 feature should be seen as
a _good_ thing.  The std::unique_ptr was so good that it made it
to the standard.  Not a small feat.  Bonus.  Once we move on to C++11
for real (I don't know when), then there's one less API to learn.

> 
>> Several others have said they think this is a good idea.  I've even
>> ran this idea through the libstdc++ maintainer in person at the Cauldron.
>> Trevor said he wants to do this in gcc as well.
> 
> I just wanted to voice my opposition, that's all.  I don't have to
> give up just because a few others think otherwise.  Right?

Of course.  The problem is that your opinion is interpreted as
a hard blocker.  The result is stalling.

>> If redirecting to C++11 std::unique_ptr turns out to cause trouble, or
>> I'm hit by a bus, then it's trivial to remove that redirection.  All it
>> takes is remove a handful of lines of code guarded with
>> #if __cplusplus >= 201103 to always go through the fallback implementation.
> 
> Once again, it's not just this single patch that bothers me.  Once we
> have enough of these #if's, removing them is not necessarily a trivial

And not "not necessarily" either...  This just looks like fear of
the unknown, I'm afraid.

> matter, especially when most of the builds, perhaps even all of them,
> have been using the C++11 code path all the time, and the other one
> has simply bitrotted.

As I've said before, we can make use of the buildbot for that.
If the fallback code breaks, you get an immediate email notification.

>>> which
>>> threaten to undo at least some part of the benefits you aspire to
>>> achieve by moving to C++ in general and to a new enough C++ standard
>>> in particular.
>>
>> I don't understand what this is trying to say.
> 
> The move to C++ was supposed to make the maintenance easier, right?
> I'm saying that targeting more than a single language standard makes
> maintenance harder, so it works against the purpose of the move.

Many, many, many, free software projects, small and large, went
through that exercise.  It's known territory, we can learn
from their experience, and we know that it's quite doable.

> 
>> And no, I'm not planning on getting hit by a bus.  :-)
> 
> Who is?

;-)

Thanks,
Pedro Alves
  

On Thu, Oct 13, 2016 at 11:16:26AM +0100, Pedro Alves wrote:
> On 10/13/2016 07:11 AM, Eli Zaretskii wrote:
> >> Cc: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
> >> From: Pedro Alves <palves@redhat.com>
> >> Date: Thu, 13 Oct 2016 02:28:44 +0100
> >>
> >> Yeah, sounds right.  I was trying get the gdb::unique_ptr in, in order
> >> to unblock the cases I suggested you use unique_ptr, but I'm a bit
> >> confused on what to do about it now...  I _think_ people are generally
> >> OK with it.  There was some opposition, but I'm not sure anymore
> >> whether it still exists.  C++11 is now on the table, but maybe a
> >> staged approach (enable C++11 while supporting C++03 too for a while,
> >> to catch issues) would make sense anyway.
> > 
> > I still think we shouldn't have workarounds for versions of the
> > standard older than what we want to support.
> 
> That is not what the patch does...  
> 
> #1 - Starting from the position that gdb wants to build with C++03
>      compilers.
> 
> #2 - The patch adds a new smart pointer type.  In C++03, you're pretty
>      much guaranteed to need to do that.  The one the standard gives you
>      is too limited.
> 
> #3 - (owning) smart pointers all look the same for most client code - they're
>      meant to look like raw pointers afterall - i.e., ptr->foo, and *ptr
>      works as expected, transparently.
> 
> #4 - where owning smart pointer APIs tend to differ is in code around pointee
>      ownership.
> 
>      I chose to model the API on std::unique_ptr (C++11).
>      There are advantages.
> 
> #5 - We could stop here.  Done.
> 
> #6 - But I went a step further, and made it so that if we have C++11
>      handy, then we can make use of the compiler to catch bugs at
>      compile time, all while keeping the nice API.
> 
> So again:
> 
> > I still think we shouldn't have workarounds for versions of the
> > standard older than what we want to support.
> 
> there's no workaround for versions older than what we want to support
> at all.  There's the code for the version that we had decided earlier
> that we wanted to support, plus extra API enforcement when built
> with a compiler that supports a version _newer_ than what we want
> to support.
> 
> 
> I'll give you more examples of why I think forbidding C++11 completely
> is short sighted.  C++11 has a nice feature where you can tell the compiler
> that some class is "final" (not possible to to inherit from that class).
> This allows the compiler to optimize the code better.
> 
> Another nice C++11 feature is that it's possible to mark class methods
> as overriding a virtual method in the base class.  This is quite handy,
> because with C++03, if you change the method's prototype in base class,
> you have to remember to update all users.  And without the "override" marker,
> the compiler won't help you find them at all.  The program builds, and then
> fails at run time.  Maybe only after release do you notice it.
> With the override marker, the compiler now errors out pointing at all
> the places you need to fix.  Bug found at compiler time.  Wonderful.
> 
> So if we have a C++11 compiler handy, why not make use of these
> features to help with development?  I don't see how it makes
> sense to not take advantage of compiler help if available.

And infact gdb already does the same exact sort of thing in the form of
the ATTRIBUTE_PRINTF macros.

> Here's GCC conditionally using these features if available as well:
> 
>   https://gcc.gnu.org/ml/jit/2016-q2/msg00000.html
> 
> And GCC still supports building with a C++03 compiler.  Just
> like GDB.  Tons of projects out there do the same thing.

as far as conditional compilation goes this seems a lot saner than  many
of the ifdefs in gcc, and probably the rest of the toolchain.  Its also
a lot easier to test than many of them.

> > IOW, if we decide to use
> > C++11, we should _require_ a compiler that supports it, and error out
> > if the configure test(s) regarding that fail.
> > 
> 
> So:
> 
> #1.1 - for people with new compilers, we'd go through C++11 std::unique_ptr
>        all the time.
> 
> #1.2 - people with older compilers would be out of luck.  tough.  get newer
>        compiler first.
> 
> Vs:
> 
> #2.1 - for people with new compilers, we'd go through std::unique_ptr all
>        the time.
> 
> #2.2 - for people with older compilers we go through the fallback
>        implementation.  gdb still works.
> 
> (I wasn't planning on starting the discussion on whether 1.2 is
> reasonable nowadays.  That happened because someone else brought it up.)
> 
> 
> So for people with newer compilers, the result is the same.
> (#1.1 == #1.2).  While I'm proposing a solution that gives us all the
> benefits (#2.1) while keeping support for older compilers
> for a little while longer (#2.2).
> 
> > Supporting more than one standard means we will have rarely used code
> > that is almost never tested, and that means maintenance burden 
> 
> That same argument could be applied to gnulib.  Or the many HAVE_FOO's
> in the code base.  We rely on gnulib's reimplementation of the whole
> printf family of functions if the compiler doesn't support it properly
> for example!

Claiming the C++98 code will be rarely tested is only true if very few
people try to compile gdb somewhere that doesn't have a C++11 compiler,
in which case yes we might as well drop support for C++98 compilers
because supporting them doesn't help anyone.  Basically you can't have
it both ways, either there are people who benefit from support for C++98
compilers, and so they'll test C++98 only code, or those people don't
exist, and you might as well require C++11.

> The sane way to make sure the fallback code still works is
> via autotesting with the buildbot.

and doing that is a whole lot easier than say testing if gdb works
without HAVE_PERSONALITY defined since testing the latter requires some
old kernel and probably glibc.

> > that means maintenance burden 
> 
> For whom?  _I'm_ willing to maintain this code.  It's not rocket
> science code.  And then users of the smart pointer don't really
> have to care.  Whether C++11 or C++03, you write the same code.
> You may miss a gdb::move call with C++03.  No big deal.  The fix
> is just to add a gdb::move call.
> 
> Several others have said they think this is a good idea.  I've even
> ran this idea through the libstdc++ maintainer in person at the Cauldron.
> Trevor said he wants to do this in gcc as well.
> 
> If redirecting to C++11 std::unique_ptr turns out to cause trouble, or
> I'm hit by a bus, then it's trivial to remove that redirection.  All it
> takes is remove a handful of lines of code guarded with
> #if __cplusplus >= 201103 to always go through the fallback implementation.
> 
> So there's no "point of no return here".
> 
> > which
> > threaten to undo at least some part of the benefits you aspire to
> > achieve by moving to C++ in general and to a new enough C++ standard
> > in particular.
> 
> I don't understand what this is trying to say.
> 
> I propose to move forward with my plan, and if it does cause
> trouble, then we drop the gdb::unique_ptr -> std::unique_ptr mapping.
> 
> It may happen, I don't know.  We will know if it will indeed cause
> trouble in practice if we try.

having delt with very similar smart pointers in other projects that
couldn't require C++11 I'll be suprised if it causes problems.

> And _in_ parallel, as a separate discussion, we can discuss dropping
> support for C++03 and going C++11 fully.  I think we could pull it
> off, but I wouldn't want to do it in a rush, and without an easy
> fallback plan available.  The patch I sent yesterday to make use
> of C++11 with gcc >= 4.8 would be the first step.  Then if that doesn't
> cause problems, we can tell the autoconf macro to require C++11
> with a single line change.  If _requiring_ C++11 causes significant
> trouble on people, and we want to go back to supporting older compilers,
> then it's easy to revert that, and go back to the unique_ptr C++03
> fallback implementation.  This is header-only template code, so we
> really can't tell if this will cause trouble for people without
> adding actual _uses_ of the smart pointer.

Yeah, I really don't see what the down side to adding the fall back, and
then deleting it in several weeks or months or whatever it ends up
taking to require C++11 is, and I don't see what the advantage of
rushing a switch to C++11 instead is.

Trev
  

> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 13 Oct 2016 15:26:07 +0100
> 
> On 10/13/2016 02:52 PM, Eli Zaretskii wrote:
> 
> > Since I'm still unclear what is the oldest C++ standard we want to
> > support, I cannot decide whether I agree or disagree.  Is the version
> > of the C++ standard we support C++03 or C++11?  My comment assumed
> > that it was C++11.
> 
> It was decided in 2014 that we'd target C++03.  We have not decided
> yet to _require_ C++11.

Then that comment is not yet relevant.  But then I think we should not
use C++11 code in GDB.

> >> I'll give you more examples of why I think forbidding C++11 completely
> >> is short sighted.
> > 
> > Thanks, but that kind of arguments completely misses the point.  You
> > don't need to convince me that a later standard is better than the
> > previous ones.  The issue at hand is whether we should write code that
> > targets more than a single language standard, where these standards
> > aren't compatible.  I think we shouldn't.
> 
> They are not incompatible.  C++03 code compiles with a C++11 compiler
> just fine.

Granted, I meant the other way around: use code that a C++11 compiler
will compile, but not a C++03 compiler.

> > The code obfuscation is very significant.  Of course, as long as you
> > don't look into the gnulib headers, you don't care, but in this case
> > you propose to have all those shims in our sources, in plain sight.
> 
> Yes, "all those shims".  One file.  And then cleaner code in thousands
> of uses of the shim throughout the codebase.  None of that
> shim-using code needs to know what version of the standard is
> being targeted.  Take a look at patch #3 from the gdb::unique_ptr
> series, and see for yourself.
> 
> So I'll take that tradeoff, yes.

Is this a one-time tradeoff, for this single patch?  Or is this a
policy?  In the former case, I have no objections.  In the latter
case, the "one file" argument doesn't apply.

> >>> that means maintenance burden 
> >>
> >> For whom?  _I'm_ willing to maintain this code.
> > 
> > Are you saying that no one else will be allowed to modify the code
> > which has 2 branches, one for C++03, the other for C++11?  I'm betting
> > you aren't saying that, 
> 
> Of course not.
> 
> > and so this is a burden for all of us, not just for you.  
> 
> You're automatically assuming it's a burden.  I believe that's
> false.

It should be clear and agreed by all that maintaining two
implementations for a single feature is more work than maintaining
just one.

> >> It's not rocket science code.
> > 
> > Today it isn't.  I'm not talking about this particular patch, I'm
> > talking about this policy in general.  As do you, I suppose: you
> > aren't just asking for agreement about using C++11 features in this
> > single case, right?
> 
> I'm talking about adding a small utility that is going to be used 
> extensively throughout the codebase.  A smart pointer is a central widget.
> That mine was modeled on a C++11 feature should be seen as
> a _good_ thing.  The std::unique_ptr was so good that it made it
> to the standard.  Not a small feat.  Bonus.  Once we move on to C++11
> for real (I don't know when), then there's one less API to learn.

And I'm again asking whether this is about this single patch, or about
a more general policy.  I assume that it's the latter, in which case
we are not talking about a single small utility, we are talking about
all the code that will be in the future admitted to GDB with the same
premise.  It is the policy that I object to, not a single exception.

> >> Several others have said they think this is a good idea.  I've even
> >> ran this idea through the libstdc++ maintainer in person at the Cauldron.
> >> Trevor said he wants to do this in gcc as well.
> > 
> > I just wanted to voice my opposition, that's all.  I don't have to
> > give up just because a few others think otherwise.  Right?
> 
> Of course.  The problem is that your opinion is interpreted as
> a hard blocker.  The result is stalling.

If there are no more convincing arguments, then a usual way out of
stalling is to find some compromise.  Is that possible in this case?

> >> If redirecting to C++11 std::unique_ptr turns out to cause trouble, or
> >> I'm hit by a bus, then it's trivial to remove that redirection.  All it
> >> takes is remove a handful of lines of code guarded with
> >> #if __cplusplus >= 201103 to always go through the fallback implementation.
> > 
> > Once again, it's not just this single patch that bothers me.  Once we
> > have enough of these #if's, removing them is not necessarily a trivial
> 
> And not "not necessarily" either...  This just looks like fear of
> the unknown, I'm afraid.

What exactly is unknown here?  It should be clear to anyone that more
of such patches, with separate implementations for C++11 and C++03,
will come soon enough.  What reason do you have to think they won't?
After all, C++11 is so much better than C++03, right?

> > matter, especially when most of the builds, perhaps even all of them,
> > have been using the C++11 code path all the time, and the other one
> > has simply bitrotted.
> 
> As I've said before, we can make use of the buildbot for that.
> If the fallback code breaks, you get an immediate email notification.

If someone sets it up to build both with and without C++11, yes.
A.k.a. "maintenance burden".
  

On 10/13/2016 03:46 PM, Eli Zaretskii wrote:
> And I'm again asking whether this is about this single patch, or about
> a more general policy.  I assume that it's the latter, in which case
> we are not talking about a single small utility, we are talking about
> all the code that will be in the future admitted to GDB with the same
> premise.  It is the policy that I object to, not a single exception.

I don't have an answer simply because I don't know what we'll
need in the future.  All I know right now that we sorely need
an owning smart pointer.  And for this particular case, I think
it makes a ton of sense to go dual dialect.

Maybe for other utilities that we may find missing in C++03,
we may decide to write replacements based on what C++11 or
later does, if they exist, and always use our replacement
code, even when compiled in C++11 mode.  In the owning smart
pointer case, there's a benefit to using the C++11 version.
For other things, there probably isn't.

So I don't plan on doing anything like this in the future.
It's hard to set a policy when all you have is one
example.

If I knew the future, I'd play the lottery.

Thanks,
Pedro Alves
  

> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 13 Oct 2016 16:04:05 +0100
> 
> On 10/13/2016 03:46 PM, Eli Zaretskii wrote:
> > And I'm again asking whether this is about this single patch, or about
> > a more general policy.  I assume that it's the latter, in which case
> > we are not talking about a single small utility, we are talking about
> > all the code that will be in the future admitted to GDB with the same
> > premise.  It is the policy that I object to, not a single exception.
> 
> I don't have an answer simply because I don't know what we'll
> need in the future.

If we agree on some policy, then we don't need to worry about the
future, because the policy will determine what is going to be admitted
and what not.

> All I know right now that we sorely need an owning smart pointer.
> And for this particular case, I think it makes a ton of sense to go
> dual dialect.

But if we agree to require C++11 starting from now, you can go ahead
with your patch, and don't even need the other dialect.  So this
sounds like a win-win solution to me.

The only other thing we need to agree is that we are not going to
switch to a C++ standard newer than C++11, and won't allow code that
doesn't compile with C++11 compilers, until the oldest compiler which
supports that newer standard is at least 3 years old (like GCC 4.8.1
is today).

Does this sound like a compromise everyone can live with?
  

I hadn't answered everything.  Correcting that now.

On 10/13/2016 03:46 PM, Eli Zaretskii wrote:

>> You're automatically assuming it's a burden.  I believe that's
>> false.
> 
> It should be clear and agreed by all that maintaining two
> implementations for a single feature is more work than maintaining
> just one.

Sure, as general principle.  In this particular case, I can't
imagine any amount of significant work.

>>>> Several others have said they think this is a good idea.  I've even
>>>> ran this idea through the libstdc++ maintainer in person at the Cauldron.
>>>> Trevor said he wants to do this in gcc as well.
>>>
>>> I just wanted to voice my opposition, that's all.  I don't have to
>>> give up just because a few others think otherwise.  Right?
>>
>> Of course.  The problem is that your opinion is interpreted as
>> a hard blocker.  The result is stalling.
> 
> If there are no more convincing arguments, then a usual way out of
> stalling is to find some compromise.  Is that possible in this case?

I don't think so.

>>>> If redirecting to C++11 std::unique_ptr turns out to cause trouble, or
>>>> I'm hit by a bus, then it's trivial to remove that redirection.  All it
>>>> takes is remove a handful of lines of code guarded with
>>>> #if __cplusplus >= 201103 to always go through the fallback implementation.
>>>
>>> Once again, it's not just this single patch that bothers me.  Once we
>>> have enough of these #if's, removing them is not necessarily a trivial
>>
>> And not "not necessarily" either...  This just looks like fear of
>> the unknown, I'm afraid.
> 
> What exactly is unknown here?

The amount of work it'd take to remove the #ifs.

>>> matter, especially when most of the builds, perhaps even all of them,
>>> have been using the C++11 code path all the time, and the other one
>>> has simply bitrotted.
>>
>> As I've said before, we can make use of the buildbot for that.
>> If the fallback code breaks, you get an immediate email notification.
> 
> If someone sets it up to build both with and without C++11, yes.
> A.k.a. "maintenance burden".

All you need is one of buildbots testing gdb on an older distro
that doesn't have a new enough compiler.  It may even be already
there -- e.g., mjw's debian build bot.  So probably zero work,
actually.

Thanks,
Pedro Alves
  

On 10/13/2016 04:19 PM, Eli Zaretskii wrote:
>> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org

>> All I know right now that we sorely need an owning smart pointer.
>> And for this particular case, I think it makes a ton of sense to go
>> dual dialect.
> 
> But if we agree to require C++11 starting from now, you can go ahead
> with your patch, and don't even need the other dialect.  So this
> sounds like a win-win solution to me.

Well, that'd be perfect.

But as I mentioned elsewhere, I'd prefer to take a staged approach
to C++11.  I.e., have a fallback plan.  My shim would actually _help_
with that.  So the plan would span a few weeks, and it'd be:

#1 - get gdb::unique_ptr in

#2 - start using unique_ptr throughout (there's a ton of work
     to do here, and it go on in parallel with the remainder
     of the plan.)

#3 - install the patch that switches C++11 on if the compiler supports it.
     The one I sent yesterday.

#4 - see if that causes problems.  fix problems.  maybe revert patch
     from step #3 if problems are hard to solve quickly.

#5 - flip to consider C++11 mandatory.  Make configure error out
     if no C++11 compiler is found.

#6 - see what workflows break (e.g., see if we need to do anything
     with some buildslaves.

#6.a - if $problem, revert patch from step #5.  fix whatever workflows,
       and goto #5.

#7 - otherwise, after some period, start using C++11 in full.
     Remove the shim and do s/gdb::unique_ptr/std::unique_ptr/g
     throughout the code base.

All the while between #1 and #7, we can progressively convert
cleanups to use gdb::unique_ptr.  Ie., we'd pipeline/paralyze
the work.

> The only other thing we need to agree is that we are not going to
> switch to a C++ standard newer than C++11, and won't allow code that
> doesn't compile with C++11 compilers, until the oldest compiler which
> supports that newer standard is at least 3 years old (like GCC 4.8.1
> is today).

Agreed.

> Does this sound like a compromise everyone can live with?

Thanks,
Pedro Alves
  

On 10/13/2016 04:43 PM, Pedro Alves wrote:
> All the while between #1 and #7, we can progressively convert
> cleanups to use gdb::unique_ptr.  Ie., we'd pipeline/paralyze
> the work.

err, s/paralyze/parallelize/ of course.  :-)

Thanks,
Pedro Alves
  

On 10/13/2016 04:43 PM, Pedro Alves wrote:
> On 10/13/2016 04:19 PM, Eli Zaretskii wrote:
>>> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> 
>>> All I know right now that we sorely need an owning smart pointer.
>>> And for this particular case, I think it makes a ton of sense to go
>>> dual dialect.
>>
>> But if we agree to require C++11 starting from now, you can go ahead
>> with your patch, and don't even need the other dialect.  So this
>> sounds like a win-win solution to me.
> 
> Well, that'd be perfect.
> 
> But as I mentioned elsewhere, I'd prefer to take a staged approach
> to C++11.  I.e., have a fallback plan.  My shim would actually _help_
> with that.  So the plan would span a few weeks, and it'd be:
> 
> #1 - get gdb::unique_ptr in
> 
> #2 - start using unique_ptr throughout (there's a ton of work
>      to do here, and it go on in parallel with the remainder
>      of the plan.)
> 
> #3 - install the patch that switches C++11 on if the compiler supports it.
>      The one I sent yesterday.
> 
> #4 - see if that causes problems.  fix problems.  maybe revert patch
>      from step #3 if problems are hard to solve quickly.
> 
> #5 - flip to consider C++11 mandatory.  Make configure error out
>      if no C++11 compiler is found.
> 
> #6 - see what workflows break (e.g., see if we need to do anything
>      with some buildslaves.
> 
> #6.a - if $problem, revert patch from step #5.  fix whatever workflows,
>        and goto #5.
> 
> #7 - otherwise, after some period, start using C++11 in full.
>      Remove the shim and do s/gdb::unique_ptr/std::unique_ptr/g
>      throughout the code base.
> 
> All the while between #1 and #7, we can progressively convert
> cleanups to use gdb::unique_ptr.  Ie., we'd pipeline/paralyze
> the work.

So from the analysis I did at [1] it seems like we're actually
clear from the buildslave's side on requiring C++11.  I thought
it take longer to update the buildslaves, but MarkW was quick
and there doesn't seem to be other buildslaves that need
updating.

Sooo....  Shall we proceed with the straw man proposal and
apply the patches at [2] (enable -std=gnu+11 on gcc >= 4.8)?

 [1] - https://sourceware.org/ml/gdb-patches/2016-10/msg00496.html
 [2] - https://sourceware.org/ml/gdb-patches/2016-10/msg00336.html

Do people feel this hasn't been sufficiently discussed?

If we can do this now, I'll happily drop my shim in favor of
jumping to C++11 quicker!  Maybe it'll find a home in gcc.  :-)

I'd love to hear feedback.

Thanks,
Pedro Alves
  

> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 18 Oct 2016 00:43:19 +0100
> 
> Sooo....  Shall we proceed with the straw man proposal and
> apply the patches at [2] (enable -std=gnu+11 on gcc >= 4.8)?
> 
>  [1] - https://sourceware.org/ml/gdb-patches/2016-10/msg00496.html
>  [2] - https://sourceware.org/ml/gdb-patches/2016-10/msg00336.html
> 
> Do people feel this hasn't been sufficiently discussed?
> 
> If we can do this now, I'll happily drop my shim in favor of
> jumping to C++11 quicker!  Maybe it'll find a home in gcc.  :-)
> 
> I'd love to hear feedback.

You don't really want to hear more from me on this, do you?  Because
what I can say is quite clear.  Right?

Thanks.
  

On 10/17/2016 06:43 PM, Pedro Alves wrote:
> On 10/13/2016 04:43 PM, Pedro Alves wrote:
>> On 10/13/2016 04:19 PM, Eli Zaretskii wrote:
>>>> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
>>
>>>> All I know right now that we sorely need an owning smart pointer.
>>>> And for this particular case, I think it makes a ton of sense to go
>>>> dual dialect.
>>>
>>> But if we agree to require C++11 starting from now, you can go ahead
>>> with your patch, and don't even need the other dialect.  So this
>>> sounds like a win-win solution to me.
>>
>> Well, that'd be perfect.
>>
>> But as I mentioned elsewhere, I'd prefer to take a staged approach
>> to C++11.  I.e., have a fallback plan.  My shim would actually _help_
>> with that.  So the plan would span a few weeks, and it'd be:
>>
>> #1 - get gdb::unique_ptr in
>>
>> #2 - start using unique_ptr throughout (there's a ton of work
>>      to do here, and it go on in parallel with the remainder
>>      of the plan.)
>>
>> #3 - install the patch that switches C++11 on if the compiler supports it.
>>      The one I sent yesterday.
>>
>> #4 - see if that causes problems.  fix problems.  maybe revert patch
>>      from step #3 if problems are hard to solve quickly.
>>
>> #5 - flip to consider C++11 mandatory.  Make configure error out
>>      if no C++11 compiler is found.
>>
>> #6 - see what workflows break (e.g., see if we need to do anything
>>      with some buildslaves.
>>
>> #6.a - if $problem, revert patch from step #5.  fix whatever workflows,
>>        and goto #5.
>>
>> #7 - otherwise, after some period, start using C++11 in full.
>>      Remove the shim and do s/gdb::unique_ptr/std::unique_ptr/g
>>      throughout the code base.
>>
>> All the while between #1 and #7, we can progressively convert
>> cleanups to use gdb::unique_ptr.  Ie., we'd pipeline/paralyze
>> the work.
>
> So from the analysis I did at [1] it seems like we're actually
> clear from the buildslave's side on requiring C++11.  I thought
> it take longer to update the buildslaves, but MarkW was quick
> and there doesn't seem to be other buildslaves that need
> updating.
>
> Sooo....  Shall we proceed with the straw man proposal and
> apply the patches at [2] (enable -std=gnu+11 on gcc >= 4.8)?
>
>  [1] - https://sourceware.org/ml/gdb-patches/2016-10/msg00496.html
>  [2] - https://sourceware.org/ml/gdb-patches/2016-10/msg00336.html
>
> Do people feel this hasn't been sufficiently discussed?
>
> If we can do this now, I'll happily drop my shim in favor of
> jumping to C++11 quicker!  Maybe it'll find a home in gcc.  :-)
>
> I'd love to hear feedback.

I personally feel this hasn't been discussed much, but honestly it 
doesn't feel like discussion is going to change anything here other than 
create clashes of ideas. :-)

I've seen this go from "You got it wrong. We're not going to move to 
C++11" to "So, shall we move now?" rather quickly. Nothing showed up in 
gdb@ either.

Since we're already moving things quickly, we should probably discuss a 
policy to accept the next standard version and follow that from now on.

My $0.02.
  

On 10/19/2016 07:02 PM, Luis Machado wrote:
> On 10/17/2016 06:43 PM, Pedro Alves wrote:

>> Sooo....  Shall we proceed with the straw man proposal and
>> apply the patches at [2] (enable -std=gnu+11 on gcc >= 4.8)?
>>
>>  [1] - https://sourceware.org/ml/gdb-patches/2016-10/msg00496.html
>>  [2] - https://sourceware.org/ml/gdb-patches/2016-10/msg00336.html
>>
>> Do people feel this hasn't been sufficiently discussed?
>>
>> If we can do this now, I'll happily drop my shim in favor of
>> jumping to C++11 quicker!  Maybe it'll find a home in gcc.  :-)
>>
>> I'd love to hear feedback.
> 
> I personally feel this hasn't been discussed much, but honestly it
> doesn't feel like discussion is going to change anything here other than
> create clashes of ideas. :-)

:-)

Right, discussion just for the sake of it is not in anyone's interests, IMO.

AFAIK, all blockers that _I_ thought existed either don't actually
exist or have been resolved.

If there are specific, actual blockers, we should certainly discuss those.

> 
> I've seen this go from "You got it wrong. We're not going to move to
> C++11" to "So, shall we move now?" rather quickly. 

To be clear, the original message, which is still true was:

The gdb::unique_ptr patch does _not_ make us require C++11.

> Nothing showed up in gdb@ either.
> Since we're already moving things quickly, we should probably discuss a
> policy to accept the next standard version and follow that from now on.

IMO such a discussion doesn't have to block starting to require C++11,
and can happen in parallel, since for sure we're not going to start
thinking about requiring C++14 right now.

In any case, Eli has suggested a policy.

Thanks,
Pedro Alves