From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10958 invoked by alias); 3 Jan 2010 04:58:00 -0000 Received: (qmail 10940 invoked by uid 22791); 3 Jan 2010 04:57:58 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 03 Jan 2010 04:57:54 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 8C5532BAB58; Sat, 2 Jan 2010 23:57:52 -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 M0FKiaampqMU; Sat, 2 Jan 2010 23:57:52 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id F1A142BAB48; Sat, 2 Jan 2010 23:57:51 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id B4ED7F5937; Sun, 3 Jan 2010 05:57:17 +0100 (CET) Date: Sun, 03 Jan 2010 04:58:00 -0000 From: Joel Brobecker To: Jan Kratochvil Cc: gdb-patches@sourceware.org, gdb@sourceware.org, Vladimir Prus Subject: Re: [patch] Re: Regression: field type preservation: 7.0 -> 7.0.1+HEAD Message-ID: <20100103045717.GZ2788@adacore.com> References: <20100101184505.GA18391@host0.dyn.jankratochvil.net> <201001021308.19130.vladimir@codesourcery.com> <20100102203022.GA8372@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100102203022.GA8372@host0.dyn.jankratochvil.net> User-Agent: Mutt/1.5.20 (2009-06-14) 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 X-SW-Source: 2010-01/txt/msg00035.txt.bz2 > gdb/ > 2010-01-02 Jan Kratochvil > > * value.c (value_primitive_field): Remove one check_typedef call. > Move bitpos and container_bitsize initialization after > allocate_value_lazy. New comment before accessing TYPE_LENGTH. Interesting. This patch allowed me to figure out why the initial patch sent by Vladimir was in fact working when it looks like it could not possibly work :). Personally, I'm not a big fan of the approach you took in this case. You rely on a side-effect of the various allocate_* routines, but this is only because these routines call check_typedef. In particular, there is something interesting in allocate_value_lazy. A variable is declared... struct type *atype = check_typedef (type); ... but then never used. There is no available info as to why this is, as this is here since day 1 of the public CVS repo, but it's now clear to me that this helps fixing that length - a type length that does not appear to be used at all in the rest of the routine! In my opinion, I agree with Daniel's comment that it is unusual to call check_typedef without storing the function result. But this seems to be less awkward than relying on an undocumented side-effect of a function that uses an undocumented side-effect of a check_typedef, especially since the function in question does not really need that side-effect to be applied at the time the function is called! So, my proposal, if the other maintainers agree, is to document the side-effect of check_typedef (sets the typedef TYPE_LENGTH) as this appears to be a fully-intended behavior, and then do: > - type = check_typedef (type); > + /* Call check_typedef on our type to make sure that, if TYPE > + is a TYPE_CODE_TYPEDEF, its length is set to the length > + of the target type instead of zero. However, we do not > + replace the typedef type by the target type, because we want > + to keep the typedef in order to be able to print the type > + description correctly. */ > + check_typedef (type); > gdb/testsuite/ > 2010-01-02 Jan Kratochvil > > * gdb.mi/var-cmd.c (do_bitfield_tests): Change "V.sharable" type to > "uint_for_mi_testing". This part is OK. -- Joel