From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9236 invoked by alias); 30 Jul 2018 17:58:36 -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 9226 invoked by uid 89); 30 Jul 2018 17:58:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_1,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=H*u:6.1, H*UA:6.1, HContent-Transfer-Encoding:8bit X-HELO: aserp2130.oracle.com Received: from aserp2130.oracle.com (HELO aserp2130.oracle.com) (141.146.126.79) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Jul 2018 17:58:34 +0000 Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w6UHnSlT193580; Mon, 30 Jul 2018 17:58:32 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2018-07-02; bh=FyV07HwhYb8jDu1UyjijQ1HCBuvx7+18XkoKIKYGH3M=; b=RMDokOgH27TKa6cTRLODFEckmxBX3R0Tiz6D5LkPHR161V8oHpsgJ09m8vhGbPQbTmqg sYeMRzX6RnU1+g3vErFcr+jMRZvv5WKtU3Epz3yEvz632MQBj0Tyt1O9xua3gaIgQDS9 gZd5y0Pemm+G6eCN5IokT8ToSEvAyt13J47tyh+tz8SZzCdrZZkjBYXbjAneT/TGi72U MCZ8NgUKSX8dbr8pV9YTfhP6bMPzGaaT1HR10i6kUBgm6afltGk3Ij4oUt3Q+3jz0EzH Qhw451eAdOTj1L9E4/4STIJrATlF2T2V9gYIhtUB5fgc589T9GBKJKdN+FKktHdB1Txs YQ== Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp2130.oracle.com with ESMTP id 2kge0cwtcu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 30 Jul 2018 17:58:32 +0000 Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w6UHwU8w027169 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 30 Jul 2018 17:58:30 GMT Received: from abhmp0012.oracle.com (abhmp0012.oracle.com [141.146.116.18]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w6UHwUek031041; Mon, 30 Jul 2018 17:58:30 GMT Received: from [10.39.216.129] (/10.39.216.129) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 30 Jul 2018 10:58:29 -0700 Subject: Re: [PATCH PR gdb/16841] virtual inheritance via typedef cannot find base To: Tom Tromey Cc: gdb-patches@sourceware.org References: <1532128565-75923-1-git-send-email-weimin.pan@oracle.com> <87y3duojwj.fsf@tromey.com> From: Wei-min Pan Message-ID: <8147f44e-5deb-9f32-76b1-ead651975961@oracle.com> Date: Mon, 30 Jul 2018 17:58:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <87y3duojwj.fsf@tromey.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2018-07/txt/msg00776.txt.bz2 Hi Tom, Thanks very much for the comments. On 7/28/2018 7:28 PM, Tom Tromey wrote: > Thanks for the patch. There are a few small issues with it. > > First, git says that most of the lines in the valops.c part have a > trailing space. Please remove those. Will do. > Weimin> + for (index = 0; index < TYPE_NFIELDS (domain); index++) > > I think the loop limit should be TYPE_N_BASECLASSES. OK, will change it to TYPE_N_BASECLASSES. BTW, there are quite a number of places that use:     for (i = 0; i < TYPE_NFIELDS (type); ++i) > And, is it possible for the desired type to be a base class of a base class? > I suspect so, but I didn't try to check; in this case you will need some > recursion here, and also I think a new test for this case would be good. Yes, I think it's possible and will need to do recursions here. Also will expand the submitted test with such classes. > > Weimin> + if (TYPE_FIELDS (domain)[index].type == curtype) > > It is more idiomatic to write TYPE_FIELD_TYPE (domain, index) here. > Also I think you need check_typedef on the left-hand-side. > Otherwise perhaps using a typedef for a base class would confuse gdb > (depending on whether the compiler emits typedefs here or not). Good point. Will change it to     if (check_typedef (TYPE_FIELD_TYPE (domain,index)) == curtype) > Weimin> + else > Weimin> + mem_offset = value_as_long (ptr); > > This line is indented too far. OK. Thanks again, Weimin > > thanks, > Tom