Re: [RFA 09/22] Remove make_cleanup_restore_current_ui

Pedro Alves
2016-10-13T10:16:46Z

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