From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id mhRuBrTGImA5ZAAAWB0awg (envelope-from ) for ; Tue, 09 Feb 2021 12:30:28 -0500 Received: by simark.ca (Postfix, from userid 112) id 0B4971EF4F; Tue, 9 Feb 2021 12:30:28 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,MSGID_FROM_MTA_HEADER,RDNS_NONE, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A95F81E54D for ; Tue, 9 Feb 2021 12:30:27 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2250F385802B; Tue, 9 Feb 2021 17:30:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2250F385802B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1612891827; bh=erOAAWS3YwFieMC8+7Du5KeG0mXvd1V3v0AWTEAd4wY=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=lvxd9hIWF1ETBHwMWSaqKMmH6ZKlp/vFEMlkpf5C5eFWcQNIpvIkaHCRnScYvUGsM 1CqkG95NOhBfShYN2dMDoIOZ098lT+6wyAcKMGGKKVFtgsX2etQ2/iMujTSDQGs26G m97JDtHzzrGtO9/n1I5+Jfcyr6aW7atOHTZLW9q8= Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2052.outbound.protection.outlook.com [40.107.223.52]) by sourceware.org (Postfix) with ESMTPS id 3CA5C385802B for ; Tue, 9 Feb 2021 17:30:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3CA5C385802B ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ApzSLOPLkN/bTvzAK5HBNzMoPG+2E8l/gFPKnq7cnbLGWAnSKzRaI5vdPixChUHpaFH2urXo6p5R4mVr5iVS/HZIz8p0l50JNazqHWdX7YTckgZn+TGGI61SNwj1k/f/kXJ6y55Gnt6pzPCC4K2WByFlSNtG9GKzzPNBwyqt2m4jrb8oui/CK8aBSpyuRJBVZwrVhTxK24pBa9SU3XD09WVDtbfrWjy1x6kXjXOIpQjk59BZnQA3XaRkg4ZPExT1kt/89YeRf3Sxyhz/VotNvE/MvkPEUPnqsDz5YcB4M2NIW5HoCM0n6Wkz2M15O4WE66QJs4lKk7mVykAjTOwRIQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=erOAAWS3YwFieMC8+7Du5KeG0mXvd1V3v0AWTEAd4wY=; b=StYYkiOMD/51kgIPqxo8ebStOdUr8GaCQoQCA1EwLafOc4ZsXMEGH3a0xCb1fTdrbwuxWuVmxzMrrFYhiRz3JORQTMe2JxXA9kR4yvGs88KNdNvH1V6rXgqsl9DDlXP/v6edGRwPC0bq9ts+YxXaqhZkRMyEftgbIHVx4UE6a5cT1e0yqIbpFKIyZxsIbEjVRMZIti+U+GrNefPupEuF7VX/MNPS+dl3eFMRQmCIDIzIXoDG+/ZNz0TaohduHRgjEb9llb3DDMN3W6kgqLBzFxzv6gD8p9NzOjdJ10+LLt/X5Daag7sOt/05oWgjVycd+GEanz+iEtdcnKmpflt2JQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none Received: from DM6PR12MB2762.namprd12.prod.outlook.com (2603:10b6:5:45::15) by DM5PR12MB1274.namprd12.prod.outlook.com (2603:10b6:3:78::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3846.25; Tue, 9 Feb 2021 17:30:22 +0000 Received: from DM6PR12MB2762.namprd12.prod.outlook.com ([fe80::31d8:f503:f7b2:f44]) by DM6PR12MB2762.namprd12.prod.outlook.com ([fe80::31d8:f503:f7b2:f44%3]) with mapi id 15.20.3825.030; Tue, 9 Feb 2021 17:30:22 +0000 Subject: Re: [PATCH 19/30] Add new location description access interface To: Tom Tromey , Zoran Zaric via Gdb-patches References: <20201207190031.13341-1-Zoran.Zaric@amd.com> <20201207190031.13341-20-Zoran.Zaric@amd.com> <87im72dyge.fsf@tromey.com> Message-ID: <82aea0d9-ee82-9f59-befe-f02a47f0d871@amd.com> Date: Tue, 9 Feb 2021 17:30:16 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [2a00:23c7:5a85:6801:b142:b2da:a877:6c3] X-ClientProxiedBy: AM4PR07CA0013.eurprd07.prod.outlook.com (2603:10a6:205:1::26) To DM6PR12MB2762.namprd12.prod.outlook.com (2603:10b6:5:45::15) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [IPv6:2a00:23c7:5a85:6801:b142:b2da:a877:6c3] (2a00:23c7:5a85:6801:b142:b2da:a877:6c3) by AM4PR07CA0013.eurprd07.prod.outlook.com (2603:10a6:205:1::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3846.11 via Frontend Transport; Tue, 9 Feb 2021 17:30:21 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 9bc85d8a-fb5b-4e0a-1469-08d8cd2060bb X-MS-TrafficTypeDiagnostic: DM5PR12MB1274: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:5236; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Q40fY0PLYhE17e2ldpM66etdtRLK1ZojPwlB+AuA12wH6Lj4WAzFWzDrg0j5wNTrmJjRqRhcf6EH1R7zmNgEp6DIkkQhVDUbuvL7mdtb0GqrO62ScvGg3RvnqIHztzjbgri+TMqRhED30cc/kheh57sOYowlP1RnuSVLi37RFB4d+K3TWpfBAFXga8WtuFDkLFvcI8bRXan62ME6y0Yv0pV/LTODnk8egC+BVuzr4AHmOlmzDHVapR+hZtq6JcS70MU+X24WzGceGQfl4KjJCX5wfniBlXbaGx1lJnlJf8px7qtBoN40pwevaplNVM5TG4e/zyTtI00kmTmMdeBmQzD+E/pfGu4yXM/ghkywnAz4Lm3ANx1JcDL2igjTdHwge/NP/HL2fgLe4ypI0/BPwRSkzNBjzRWmlv6MyuFtUysrB13WJ67xFA96/Z/ZJSfbizHfESs7gWBv67rC84AzjVk9u0H+Jk6A2o5DTDoNxukzUxEyHlqjViQBU3365So1PphUyklVWyP/yaVrHu5NSSvJj6RqeNMYH991f/lKQZqMpoxLH2SnY4LrDAYHpFNAv4odvGIV20/XRO85+HuDzJhsYlN8glaQjrrhUutkKu0= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR12MB2762.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(39860400002)(376002)(346002)(396003)(136003)(366004)(86362001)(36756003)(8936002)(478600001)(186003)(16526019)(83380400001)(6486002)(31696002)(66946007)(2616005)(66476007)(66556008)(2906002)(31686004)(52116002)(6666004)(8676002)(110136005)(316002)(5660300002)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?alY1V3pKZVgvMXRhVkdOSWRTUWF6OFEvdXBaallNbE9kYXpMZUw3QmV6Z2JT?= =?utf-8?B?aDNmZnVRc3d2b3JvMFlXVEdpYlQ4VGdtdVpJWnZjd1ArY0J1Uk9XRmdOTFEx?= =?utf-8?B?aDFGamZqZ3o3bVJDUjZ6ZmVjWmQxMno4dlIzWmh5cmJtTnVEWU9YekNkalBH?= =?utf-8?B?ZzcyWXQ2cTlsZGVCanRpa082R0V1TEI1TkNEVmpmM2xjRktJRTBUODQ4N0xB?= =?utf-8?B?QnhoSDVVSHNkcHl0OHVlK1VsU2FiQ3ltaXpVWlEyNHNyUjZ3b0Y5VmJ1SWhR?= =?utf-8?B?bWRmZ0d5VExISU1wTUoyUEcvblRQL0VTc3JpTHlDRE1KZUhjQlRjNytILzlO?= =?utf-8?B?d2p6K3dsZVZsSlpKM0dXZG04UUw4blBQa2JsMDlYc1JWYkVwVDVOYVQvbytW?= =?utf-8?B?VjJ1akdHYWVQUU9lR3hoWDRPcXNKTURqeDFsVGNWdDdQbStiM0thcS9UVHJ3?= =?utf-8?B?Qm9RTEpiOFRVQXBNSkZSOG1DUGVFei9yWTFxclAyTkRnZHBacnpxc0I1WSta?= =?utf-8?B?TWg3dkt4KzBwNk9aYkRmdkhpbmFxb2Eyc2FPZGZNcEFXcmZwelVvS0dwTGN0?= =?utf-8?B?RVQyK3h1cklqSVMzSFVwQk1sdEc5MFFJbG5YNGJkbjIxOGp5NUlKUWpaeGRK?= =?utf-8?B?Ryt0VTI2OHo3WEJTSk5yQTJOTUVjTlE2SEk0Q25OVFRsNGZaWUkzc0o0ejdS?= =?utf-8?B?ZVNNU3VNSGdaSVpSQTRwa0Y5OEtTdUp5OHpHa0VRb2t1T0VVQ1d3Ym5VMlpL?= =?utf-8?B?MDY5ZmJEMDIzdWFyeHJaZzdiN3lnYk5mVnNGQUlBeFNmeHBHcHVtck1kMnZL?= =?utf-8?B?cHJUaUhwYVdrdGtkN3ByUktXMnpWdW1SbkJFTWR4VUR5bHJBak40dnhOeGJo?= =?utf-8?B?WkNHL20vR2hUcTh6NHIvU0EwSFRDZ1BkcFh0UithbCtOajhPSmRiSUM4U0RD?= =?utf-8?B?ZDNCcXVSQVRZN1BWMVBzcDl1NlBISDd3N1VNKzBmZFZZWEl1bmZLMVEyOWE0?= =?utf-8?B?VDhWanY5ZnpZS2kwVW8yb1NldUtPR0Z6SFVZdnp1VjI1bzA5N084cEUrejNw?= =?utf-8?B?SDJzTUVlTWxxbmtORVlpMHdUVVVXeVZDbUMvZ1crSUtxQXhCblcyNmk4dG1H?= =?utf-8?B?QzNGellxcXZmTlFucnN4YTR2eTNQdlVweW5Rb3J2dm5NNnBRSlhLQUR1NTE2?= =?utf-8?B?TVBxRkFLak12dWg2aWxCMFMvOUE4TWxWc1huN0hhUkRwd0FLRkxZR2kzVkIr?= =?utf-8?B?Uk1TUHV0d0hrM2lyNWFxWENNbm02VDlVaWZQQ2dPN3kvb2J2dmVFZVNaSFBj?= =?utf-8?B?ZDBIVFFBam1YQ3lwbjdROUt1S0tkOWZIVzAyL3Y4bHhjVVUzcHB3Ym5ua0o2?= =?utf-8?B?ZnIrbS9ydmYwR3NIOEt1R2VHOXFlYXlhczFDY0ZWck5kVlVnTTJuYzljcklj?= =?utf-8?B?bGE4YnBtRmVJUEpnZHhrUFJ6T2pzaEVsdUp1elhkWDMvdC9jbFdBYzF3N3l0?= =?utf-8?B?NHJFY21UMW9wNk90Q0gvZEZrLzlYaHpRVkxGUlBOZFlRcTRwUjg4a0l5US85?= =?utf-8?B?Q3dLejhEd0RQYTNTanQvNStpSndZa2M3bDM0R2Q0NlMyOUg5NGxVTW1sbGNn?= =?utf-8?B?TGpTV2VVK01MZjYzYW5nSmszdGdzZysrRTNzMU0rM2k1MDFuSEIxVlRHTUl0?= =?utf-8?B?ejRkamM2N3N6MFVCczdkYzlKejd0aVE5ZUIxL2IvRm1yRU5ONkVGMGU2bm5K?= =?utf-8?B?aHlmZGsrSnEvRGxRM3h2dVRtVzFiS1J0cXVIT1E3NjIzSFFoYnBiRE54cXE1?= =?utf-8?B?Z2tSb0xzdFpLeEZ2cTN4dXMzN2pqampEZFpxc3c5TjUydUpCQ05MY3dwekpI?= =?utf-8?Q?yHrZTq1SRSsBt?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9bc85d8a-fb5b-4e0a-1469-08d8cd2060bb X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB2762.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2021 17:30:22.0094 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: zXIupNr3l1jVuHm21kNYAHDFiYhEz9QKDIdKY/8XIH1/GqOkQdRwUf8HyiOynSbcudTrFPJWznxbzf16Qpx74A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1274 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Zoran Zaric via Gdb-patches Reply-To: Zoran Zaric Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" >> > Thank you Tom for the review. > > To be honest, I am not sure how to do this with virtual functions, > considering that the information held by the different location > descriptions categories are so very different. > > The only way how I can see this being implemented if I encapsulate all > the specifics of dealing with the different location descriptions > (reading, writing, conversions between them and the composite/implicit > pointer infrastructure) inside of that class hierarchy and change the > dwarf_entry class to have a bunch of virtual functions that only make > sense for one location description category. > > The reason why I didn't implement it that way in the first place is > because I was following the existing model of how the struct value > handling was implemented, where instead of dynamic casting there is an > enumeration check to figure out what is described by an object. > > Don't get me wrong, I am not really against that change, but it would > require a bit serious rework of the implementation. > > So if you really feel that this is a deal breaker, I could give another > crack at it in the next version. > > Zoran > After rereading my response, I realized that I probably need to explain my thought process a bit more. In the original code there were large switch cases that dealt with the evaluation result (aka location description) based on a result enumeration. This was done in two places: - at the evaluator caller side and - composite/implicit pointer callback functions. I first moved those pieces to be at the expr.c side and then when I switched to the new classes, I have replaced the big switch statement, with the if/else statements that used the dynamic cast to cast to the appropriate class instead to referring to the enumeration to know which data members are valid or not. So in a sense, the ugly code was already there. Not to mention that the struct value infrastructure is based on the same model. Originally, I had an enumeration that would be checked inside of the dwarf_entry class and return an object of the correct class but that was functionally the same as having the dynamic cast so people advised me to just use the dynamic cast because it was already used in gdb. So Tom, when you say that the type-case looks bad, do you mean just the fact that I am using the dynamic cast or the problem is more general because the dwarf_entry class is supposed to encapsulate all that logic? For the dynamic cast issue, I can always switch back to the enumeration methodology that I've described and is similar to what was there before. In the case of a potentially better design of the dwarf_entry classes, I couldn't agree more. I was trying to first mimic the previous mechanism so that people would find it easier to understand and accept. The proper encapsulation was planned for a later time, but to be frank the struct value requires the same type of remake which is not a modest task in either case. Hope this makes more sense, Zoran