This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] (Ada) fix breakpoint add on inlined function using function name.


> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]