From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 53153 invoked by alias); 20 Dec 2017 11:51:33 -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 52878 invoked by uid 89); 20 Dec 2017 11:51:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Dec 2017 11:51:32 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 0228A1170F1; Wed, 20 Dec 2017 06:51:31 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id JZEDVccAUKdt; Wed, 20 Dec 2017 06:51:30 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 945831170F0; Wed, 20 Dec 2017 06:51:30 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 7E396809B4; Wed, 20 Dec 2017 15:51:26 +0400 (+04) Date: Wed, 20 Dec 2017 11:51:00 -0000 From: Joel Brobecker To: Xavier Roirand Cc: gdb-patches@sourceware.org Subject: Re: [RFA] (Ada) fix breakpoint add on inlined function using function name. Message-ID: <20171220115126.tohoo5zpbzzmme2s@adacore.com> References: <1513765319-17005-1-git-send-email-roirand@adacore.com> <20171220112322.zh7qawogr6psip53@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171220112322.zh7qawogr6psip53@adacore.com> User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2017-12/txt/msg00446.txt.bz2 > Other than that, the patch looks good to me. But give others > a couple of weeks so they also get a chance to review the patch > as well. Here is what I propose: Send a v2 patch, with the revision > log fixed, so we don't forget about fixing (revision logs cannot > be changed once pushed without rewriting history), mentioning > that I conditionally OK-ed the patch but asked for us to wait > another couple of weeks to give everyone some time to look at it > also. > > It wouldn't hurt as well to confirm to us how you tested this patch, > and on which platform. A couple of other suggestions: 1. I'd like to suggest that we remove the "(Ada)" from the commit subject; Although, the fact that we can only see this issue with Ada as of today (based on the assumption that inlined_subroutine DIES are nested inside the subroutine that inlines them, thus making Ada the only language which has visibility over those from outside that subroutine) does not change the fact that the fix is language-agnostic. So it is conceivable that this becomes relevant to other languages at some point. 2. Add a "DWARF" to the "RFA" tag, so as to make it clear right away which area of the code this patch touches. Eg: [RFA/DWARF] fix breakpoint add on inlined function using function name The goal of these two suggestions is to quick inform the readers and reviewers that this is not a patch which is purely constrained to the ada-* modules (which people typically leave to your Ada maintainer to review), but a patch which touches common core modules of GDB. I think it has a higher chance of attracting a reviewer's attention that way. -- Joel