From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36492 invoked by alias); 11 Oct 2016 18:21:28 -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 35566 invoked by uid 89); 11 Oct 2016 18:21:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=massage, goals, labor, structs 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; Tue, 11 Oct 2016 18:21:17 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 D23773D943; Tue, 11 Oct 2016 18:21:15 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9BIL17B010636; Tue, 11 Oct 2016 14:21:02 -0400 Subject: Re: [PATCH 1/3] Introduce gdb::unique_ptr To: Luis Machado , Eli Zaretskii , Joel Brobecker References: <1476117992-5689-1-git-send-email-palves@redhat.com> <1476117992-5689-2-git-send-email-palves@redhat.com> <20161011121639.GE3813@adacore.com> <68fc02cb-59bc-012c-d1be-b5ed2076d6a5@redhat.com> <20161011144741.GF3813@adacore.com> <83insydifw.fsf@gnu.org> <4bdcd06e-1324-db5b-2c49-941a7dcfaed6@codesourcery.com> Cc: markus.t.metzger@intel.com, gdb-patches@sourceware.org From: Pedro Alves Message-ID: <45a6ac91-71f6-8e2a-b9a7-72609145564c@redhat.com> Date: Tue, 11 Oct 2016 18:21: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: <4bdcd06e-1324-db5b-2c49-941a7dcfaed6@codesourcery.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg00272.txt.bz2 On 10/11/2016 06:14 PM, Luis Machado wrote: > Some of Eli's points resonate with me. It seems we just recently got the > C++ compatibility sorted out and we're quickly moving on to try and > C++-ify what we can with no clear goal, priority list or high level > picture. So, just to make it C++ now that we require a C++ compiler. It's all described at: https://sourceware.org/gdb/wiki/cxx-conversion#Transition_plan One of the goals right now is to replace cleanups with something that the compiler does automatically. That is, completely eliminate the "forgot to call do_cleanups" bug by design. That generally involves: - replacing cleanups with either RAII objects (Tromey's scoped_restore class) - converting structs to classes with destructors. Make some classes be value objects allocated on the stack. - make use of std::string, which tends to simplify the code significantly anyway. > > I thought i'd throw this question out there. Wasn't the goal of moving > to C++ to help the implementation of features that were hard to > implement with C like some of the Fortran bits with dynamic arrays? Also > improve the poor experience with C++ debugging? All the above. Want to lend a hand? :-) See also: https://sourceware.org/ml/gdb-patches/2016-09/msg00162.html for another example. > Another point i worry about is that we will switch the focus, at least > for a few months, to C++-ifying things "just because" instead of taking > the opportunity of such a big change to review bits of GDB that need to > be redesigned/dropped/go away. Maybe just converting existing things to > C++ is not the right answer, even though it is fun to do and will > provide enough hacking fun? Well, IMO, it makes sense to first C++ify the code without changing much the design, and then redesign on top, because it'll tend to be easier once C++-yied. Also in order to do "atomic" changes; one concern per change. Another point is that I've heard time and time again (and have experienced it too) that redesigns don't happen exactly because the code is hard to massage! E.g., raise your hand if you've ever thought that it'd be nice to just use inheritance for something but didn't because doing it manually requires so much manual labor in C, and so you ended with some quicker hack adding to the mess instead. Ever looked at gdb/breakpoint.c, for example? (Rhetoric, I know you have ;-) ) Back to cleanups, here's an example (from patch #3) of how the code ends up so much more readable when using modern practices: @@ -95,14 +95,9 @@ evaluate_subexp (struct type *expect_type, struct expression *exp, CORE_ADDR parse_and_eval_address (const char *exp) { - struct expression *expr = parse_expression (exp); - CORE_ADDR addr; - struct cleanup *old_chain = - make_cleanup (free_current_contents, &expr); - - addr = value_as_address (evaluate_expression (expr)); - do_cleanups (old_chain); - return addr; + expression_up expr = parse_expression (exp); + + return value_as_address (evaluate_expression (expr.get ())); } I've mentioned this before, but I'm a big believer in keeping the code base inviting to work on. If there's people willing to work on cleaning up old ugly code, why not? If something breaks and we don't notice it, I'll blame it on lack of testsuite coverage. :-) > I have another point about whether this will stimulate more contributors > to send patches to GDB or not, but that's enough rambling for now. :-) Seems to be working so far, IMO. :-) Thanks, Pedro Alves