From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15637 invoked by alias); 13 Oct 2016 10:16:46 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 15475 invoked by uid 89); 13 Oct 2016 10:16:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=science, teach, Starting, tough X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 13 Oct 2016 10:16:30 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A77776330C; Thu, 13 Oct 2016 10:16:29 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9DAGQkT026706; Thu, 13 Oct 2016 06:16:27 -0400 Subject: Re: [RFA 09/22] Remove make_cleanup_restore_current_ui To: Eli Zaretskii References: <1474949330-4307-1-git-send-email-tom@tromey.com> <1474949330-4307-10-git-send-email-tom@tromey.com> <2f79a489b9090701f15fc04e0017c236@simark.ca> <87y41xd0dt.fsf@tromey.com> <1f5898b8-6be9-48e6-4312-72ec90e7810e@redhat.com> <87insxmbnd.fsf@tromey.com> <2855e2ec-46dc-71b7-9943-8edaebcbef90@redhat.com> <8337k0aicw.fsf@gnu.org> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org From: Pedro Alves Message-ID: <6f27db2e-4b88-b72a-5836-080c3583eb7f@redhat.com> Date: Thu, 13 Oct 2016 10:16:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <8337k0aicw.fsf@gnu.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg00349.txt.bz2 On 10/13/2016 07:11 AM, Eli Zaretskii wrote: >> Cc: Simon Marchi , gdb-patches@sourceware.org >> From: Pedro Alves >> 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