From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16416 invoked by alias); 22 Aug 2002 21:09:41 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 16407 invoked from network); 22 Aug 2002 21:09:40 -0000 Received: from unknown (HELO localhost.redhat.com) (216.138.202.10) by sources.redhat.com with SMTP; 22 Aug 2002 21:09:40 -0000 Received: from ges.redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id A0B283C48; Thu, 22 Aug 2002 17:09:37 -0400 (EDT) Message-ID: <3D655311.6030603@ges.redhat.com> Date: Thu, 22 Aug 2002 14:19:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.0) Gecko/20020810 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Daniel Jacobowitz , David Carlton Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] dwarf2read.c: set TYPE_DOMAIN_TYPE correctly for methods References: <20020822204211.GA31727@nevyn.them.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002-08/txt/msg00714.txt.bz2 > On Thu, Aug 22, 2002 at 01:35:50PM -0700, David Carlton wrote: > >> I've figured out what caused the regression that I turned up in PR >> gdb/653; here's a patch that fixes it. >> >> David Carlton >> carlton@math.stanford.edu >> >> 2002-08-22 David Carlton >> >> * dwarf2read.c (dwarf2_add_member_fn): Add back in the type >> argument that was deleted on 2002-06-14: it was needed after all, >> as PR gdb/653 demonstrates. Update call to smash_to_method_type. >> (read_structure_scope): Update call to dwarf2_add_member_fn. > > > Can you explain why this is necessary? I could not find any path to > that point where type != die->type. Just a general reminder for people comming up with patches. The place to put comments explaining changes is in the source code. Something like: /* NOTE: cagney/2002-08-12: Replaced a call to regcache_raw_read_as_address() with a call to regcache_cooked_read_unsigned(). The old, ...as_address function was eventually calling extract_unsigned_integer (via extract_address) to unpack the registers value. The below is doing an unsigned extract so that it is functionally equivalent. The read needs to be cooked as, otherwise, it will never correctly return the value of a register in the [NUM_REGS .. NUM_REGS+NUM_PSEUDO_REGS) range. */ (although, yes, the example is a bit overboard). This might feel un-natural but it is correct and very very important. The ChangeLog need only state what changed, not why. Someone studying the code and trying to figure out why things currently don't work should be able to do so by just examining the current code and its commentary. Hopefully that commentry will explain what was tried in the past and why it failed or why it needed to be changed. Oh, and adding more comments to the code is always ``obvious'' :-) (Is there a way to add ``comments'' to the doco?). enjoy, Andrew