From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10on2059.outbound.protection.outlook.com [40.107.93.59]) by sourceware.org (Postfix) with ESMTPS id 581CA385840A for ; Thu, 19 Oct 2023 15:20:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 581CA385840A Authentication-Results: sourceware.org; dmarc=fail (p=quarantine dis=none) header.from=amd.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=amd.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 581CA385840A Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.107.93.59 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1697728818; cv=pass; b=EF+EaoeTgWGl5U8upfWsRfZG7xRS7V7q0+4hxp89qXmmTWfAj9q8V32Sas53PN6iSFKdvnh5iEXk3GNwMFQERMUlcg1cg8/95L9gnETEcug11yQmZGT+VIkS6oIQrHvkGJrfTlh5f/nKru3UKdK7WXDioQKQDEr/9oyPOW8TriI= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1697728818; c=relaxed/simple; bh=rnS4CGelMu3L9BPLpBcIkcsIOp8kTxBSMI5H7A93fJ8=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=J1MHQNhZhqTbAJr4CDdGvCCeXJcEORdMlvzyBMsqzf21foBJmDkmI69D3B6RI3W2vL+EYkLJvEgWRjT3szOqs4LINXeJWoC/4TcIO7kf2GwH8B29rOPyDjLpldncSVaI+Wc4teIa6yjSz2XS3jS8he7WSppRR1+c/ByRjSf+8vo= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kFIcZ8dR45W+BKo5WZ3tQJv+otAFV8pyp+3F5F8187ezMjhWm6UFK9qPv61OBP3jbAGwDzrjCjlNNt1kcKAL1HVq2jHZ4i6L7Ea1+E74CRVHZKcvSA9gzQh9koc1E/vfHAFqKv49+L4IAFTa+oyQB2sS8kIvwQNQV1tneLWrqrtU6zVMPjt0Mh93109LHMqeQYDlHU33EWpoNcZ6+k9gr9f6F/3rosp8W7E/EBqLKyVU3gJCFd+IOPD1t5d6dlCZuFAmJwWV83EXHDMEQocU/yMNyHIcQKvObO7HRm+V9rUwtwIfLYHStSEt+KEsINJuDiuLVZb/JhULr/z2K8w9aA== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=xW4WiDqFul245yL9NxmZoO5M7KcueAGOAg7vqHFQVmA=; b=X63+QWffXpoXgVdhlmTcSwb3sgDEWtgxesrw0RiHdtZWqGoqBl4S/gbZSlpYJfl388KkiZk04FtuXO3WfYC9tKLziwDPbkZnTS8lS7KkynCUkch8deo8wsenHpLjXwz3DLv/JgVIvpl9n7Axa02bBZx1K00SC/tiD1hQ3kJcuGzWQWv0jLP6GI7QzeX0hh7S+H8WyQw+m1vV/+/A5l34q0XEOrU2unWj5t5SwpRI5sHS2Ybzvc0QGKksT3Kd8SGgQaBn+jtkuPCetNKI+ktYyXMlknwKny//B1fta7+GVi3kXqAGvZzwCovaxw1K9KydRXxdbRiqB0L52ETstCsDWw== 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 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=xW4WiDqFul245yL9NxmZoO5M7KcueAGOAg7vqHFQVmA=; b=Doao/0WcxU9UrIjy0ftdKfI4zcL1jKdWf8cKoB9G77I1dTDL9lnTCgXR/0DO2N5iUGQw9dWJWYo+P/tEtHZ7Vp5XJxryywNtx61bQVSMof9Mft1YBiFmlyhYF+sf5Te2w7dQY6xwwDclCQXHNOHybLjO670DSDAT91rOsiz8f/Q= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH3PR12MB9079.namprd12.prod.outlook.com (2603:10b6:610:1a1::9) by SA1PR12MB6894.namprd12.prod.outlook.com (2603:10b6:806:24d::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6886.34; Thu, 19 Oct 2023 15:20:13 +0000 Received: from CH3PR12MB9079.namprd12.prod.outlook.com ([fe80::3120:8014:c770:9f8f]) by CH3PR12MB9079.namprd12.prod.outlook.com ([fe80::3120:8014:c770:9f8f%7]) with mapi id 15.20.6863.047; Thu, 19 Oct 2023 15:20:12 +0000 Message-ID: <305175fd-a8d4-43f6-b6d2-8cab2aaff7a0@amd.com> Date: Thu, 19 Oct 2023 16:20:06 +0100 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 4/24] gdb: replace some so_list parameters to use references Content-Language: en-US To: Simon Marchi , Simon Marchi Cc: gdb-patches@sourceware.org References: <20231010204213.111285-1-simon.marchi@efficios.com> <20231010204213.111285-5-simon.marchi@efficios.com> <20231019110750.34bk5fflxalsb4tw@khazad-dum> <560f3443-5089-4af2-94d5-efc7bbd92684@polymtl.ca> From: Lancelot SIX In-Reply-To: <560f3443-5089-4af2-94d5-efc7bbd92684@polymtl.ca> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: FR2P281CA0022.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:14::9) To CH3PR12MB9079.namprd12.prod.outlook.com (2603:10b6:610:1a1::9) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH3PR12MB9079:EE_|SA1PR12MB6894:EE_ X-MS-Office365-Filtering-Correlation-Id: 8a11cacf-9674-4997-4cfa-08dbd0b6e3a4 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: j3apkhOZ/pRj4SJGZcA3tIcfVdphSZokzR154Qb0Dzc5/RM6hoPwqo76wJDTAfbTTAwWXbnLKRG5n+/PBFS+UJJ0h+GAtdYE1DRxzv0BzQ/mqtbAAr0z/jKdJJxpyqMgyoTXedm0F85extc/VyOlKH7nHczzSBv6zoXTkPylUABTAIUm2LyVLQcIcqcdXsQCCh1rRMESHgyyDrTs9qlJIMJnL5xSYTT/f47z7tpFeD8QF9NPfPlcX49rqX572ibwL0oObhH7u+i34JpNe1KNpghQ310l/b/6eFiVUT8ATMwHOr14jG0HWsmA/IvguTgVaA9qDrQSXF+K+85IPPNeSCMJ8VgXN0UtXnoIs+tyZ/65e3/1TB7PN5RJQmz7lbxhoWjQmMP2LEBNTy09/ArflhlKuVBV7APpw012VvZHOronoZA1sOBr/TIRNM8UpsGvniwiZ+wsCiq1ceE0LY/X1TKJlXY3VmVHUl/iWr2qhLfyby47RsOnsGy30FHRygUaGmmKQt4Q0CU7nofWKqbjFNbeI11tsRJeVaqRuKMwqjmVuvY7kS/Ishz7C/Hoao1CURHN/u6UkgHcGRlcdldBCEEXOwcsA27DAHQSUio2I5QmixeRqN4r0TTqzvCSkZlkDaCQSigYqTn4aUGqIv7gIw== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CH3PR12MB9079.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(366004)(396003)(346002)(136003)(39860400002)(376002)(230922051799003)(451199024)(64100799003)(1800799009)(186009)(31686004)(6486002)(36756003)(316002)(38100700002)(83380400001)(6506007)(6666004)(66946007)(2616005)(66476007)(66556008)(86362001)(31696002)(478600001)(110136005)(26005)(6512007)(5660300002)(41300700001)(8936002)(2906002)(8676002)(4326008)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a2FONFhKUDdjQ21aNWVCNUdrZnArdngzMWdsQjBvY3MrOHhPZDRVaHdCMkg1?= =?utf-8?B?OFNZdGk0cWthdVQycTRJeEVkWXp0b3JsTGZ6cjNKUURFbUNjdkZmQ1BhNmxN?= =?utf-8?B?YW5wU1p6a0dJa1ZRNyt3VEsvMDFyS3ZVRjViWHBtMkNUd045ZDhOYmRjNmZN?= =?utf-8?B?QmZrc285TGdrTU5Zc1RBaHBsRUpKRHZKazZQUkNxTzhtUVhmVEluaU04Ulln?= =?utf-8?B?bDdldm1pSDArTm9iWEQ3bHBVQjhVWWE3LytJMnZqSGhyc3NRbUZtRE56bklC?= =?utf-8?B?WGJKdUxLaHRJcitKT25sR3EyOWkvQksvbEorbE82VHk1SVhucTF3bFllWlh6?= =?utf-8?B?TENsUXkzRXhwR3B4WVRnQnNTZUNrVWJnNUl0NEtWN2dYTldONk4rTGFuVVNR?= =?utf-8?B?eStQWkFjeER6d2Z6ZHZXdGs4SDg5QUx3VXhGK2xUZG1xNTNsd2FJOVQ3S3Jj?= =?utf-8?B?MTg3akNad3p4SWxrSXNJMTFrOU1BcXVKa0Q2elpSN1RJWVlYR0tkTWIxK0ww?= =?utf-8?B?VlpobzVlU24rR0VDOEw3Q3V6cndmZTM0RDN5UU1CZmhkRmYzWExuSkQwekhG?= =?utf-8?B?VlVTcXFMcnk4bGoxbW9LUlZGMmMrVUpFNno5NDRYemI2NkZDYXFIbzB0SjVZ?= =?utf-8?B?b3daRUNMSVlBT1V1YklKa0g1YXRsUkN1UkhxRzJiOEdLekIrR2ptanhudW55?= =?utf-8?B?ek0xbWZleGpLM090akF2MEwyOHhCcUZJL2pOOFZreDZKUWMrR3c5WEIxcVdl?= =?utf-8?B?NXhVelkvTHNTdFdYYzQ1aGhJa2ZDR0h6VlpDbjFFV3NQUzJRMjNJQnRrdkpz?= =?utf-8?B?b2hZSGdhSm9YVmdTQ1U0a3lENjQzb291cVhsY0lVU050WHNwQ0x6a2hVN3JT?= =?utf-8?B?OXFEcjFIYW00Wm9BcjlabGtId1ZMVVRhbkNzbGxxSDAyNERKSE9LZnVmaXM3?= =?utf-8?B?U0JhM0pUNlZsYitCbmlIb0pCd1pHSEFpM3k4dTIyazJBdW40UWJKMDJuVjh0?= =?utf-8?B?MktUTDU0TDNBUExEaFIra1oyUnpSU0ZRMVIrSXVJS1lYeWpPMkxJZEZTcEdZ?= =?utf-8?B?WkltWnN6YkIwRzVYckVDaVVLZXZjNVdtb3NaMEphUk84M2ovOFZHTXNFaVlD?= =?utf-8?B?WE1SRjM3dUU0djBUOERLM25ONVVIdGRxNTN1eTdESGhuSmZqM0RNMThvQkww?= =?utf-8?B?dHVPQllDcS9seHBGdG04UDRwanhMSFFHTlphbyt6WFhmQi95WTY0TFAzcVVr?= =?utf-8?B?Rk1oU05xQ29LUkxJSEFGRFM3SlRWMjJoenV1RjZxRGE1ZDVyWlF1MzBLZEhh?= =?utf-8?B?MkJWWHYwODRncUF1cWpMZGNjYkZLWi9lQVBPWHZmSE0rc1NBK0VZcjUxMkZ4?= =?utf-8?B?MGlqZjRRRG5XSGNWU3F5YnZpWEl2YXUvRU92N3VHK3Q2T3dDMWhYR29ROWh1?= =?utf-8?B?ZDR1aHEyRVcwVUFFM3c5TTVYQnlabHk1K2RJVFRBY25lTitydkpZQTRCQmQv?= =?utf-8?B?YUdTUXAwSjM0dW0rMlRaSUlDaTZPS0dqVHNBS3F3aFAyL2RWWUJ3MFIwbXgw?= =?utf-8?B?cjBUdnZQU0pKOXFDK2RxUlVFQW4yN3plQTFOeWNRTU1SUmhjMDBNdnNDWkFi?= =?utf-8?B?cmZoNThVa0RaSGk5S3J1d3BjSmVhM3JodnZ3TUpzaDJ4dDdOYk9VWklkRmIz?= =?utf-8?B?d244NjhoVFp5ZGJuYmozd0JLZzR4b293ZHJ5ei9RMzFWVUh0VmJic05wMjJ6?= =?utf-8?B?ekdRdVBnanl1NGhqTGhKdnJiQWFOaHdnV2QxUytPNzV4bjlwckl5ZUdnbUla?= =?utf-8?B?Wjd2dzNkdDVUT3pOWGd3Q0NNUGdlQmZQRk1TK3EzY3JQSFFXeEZnQ2NaTHE0?= =?utf-8?B?aTFZQithS0hJMDVtcGxXUWM5NE1LcmMwZ0FteklDeEsyRjlCSW1OdUxlTUZR?= =?utf-8?B?ZXdGS0s5MDg5T2JwMlE1R2c2NEJiZ2QwdzZ2Nmd4U3o0S29iRkIzdFQwdmFS?= =?utf-8?B?N2M4by9xMmFBdjA2Snd2VWFySTlKdVJhT2tXYjV4NEVmaGV5a0NBWlRsc3Ns?= =?utf-8?B?RklFY0hOYndpM3N0VllaOTZ0dVRvUHNEU1hjTzFrdGd4all4aHMxTmwxa0o0?= =?utf-8?Q?gfwSyV8lHxaBSuqqcGm4Ywp9b?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8a11cacf-9674-4997-4cfa-08dbd0b6e3a4 X-MS-Exchange-CrossTenant-AuthSource: CH3PR12MB9079.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2023 15:20:12.7196 (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: TJSr/OKwU3SptphW0Mi35Hfk5qwChna09Q7qV8ZirfRMK/h7q2CCdQ4M9LBExj4OOh7ydXpDmYjGD1FJVAW0Bg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB6894 X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: >>> - void on_solib_loaded (so_list *so) override; >>> - void on_solib_unloaded (so_list *so) override; >>> + void on_solib_loaded (const so_list &so) override; >>> + void on_solib_unloaded (const so_list &so) override; >> >> It is orthogonal to this change, but it would make sense for those >> methods to be const as well. >> >> Doing this requires interp::interp_ui_out to be const as well (as done >> in attached patch). > > Why do you think the interp object should be const (which is the effect > of marking the methods const)? These methods are notifiers to let the > interp know about certain events that occured. Whether they should > modify the state of the interpreter or not is up to the particular > implementation of the interpreter. Perhaps you're right, but I don't > see it. > It is just that current interps (MI really) are just forwarding notifications. The event does not change the interp's state. Does it has to be const? No. But usually I tend to make things const unless there is a reason not to. That's all. That being said, making only those const would become quite inconsistent with the rest of the interface. So in the end, that was not a good idea! > In your patch, you make interp::interp_ui_out const, but it still > returns a non-const ui_out. So when outputting something, the > interpreter itself is not modified, but the ui_out behind it is. I > presume (sometimes) the ui_out needs to be non-const, since for things > like pagination it clearly needs to keep a state. Design-wise, is it > "correct" to make a const intepreter able to return a non-const ui_out? > I don't necessarily see that as an issue, the ui_out might need to maintain its own state. Anyway, since it is not really desirable to change this part of the interface, I think this comment can be discarded. >>> void on_about_to_proceed () override; >>> void on_traceframe_changed (int tfnum, int tpnum) override; >>> void on_tsv_created (const trace_state_variable *tsv) override; >>> diff --git a/gdb/observable.h b/gdb/observable.h >>> index acb05e9b535c..5ed6ca547ce0 100644 >>> --- a/gdb/observable.h >>> +++ b/gdb/observable.h >>> @@ -99,13 +99,12 @@ extern observable>> /* The shared library specified by SOLIB has been loaded. Note that >>> when gdb calls this observer, the library's symbols probably >>> haven't been loaded yet. */ >>> -extern observable solib_loaded; >>> +extern observable solib_loaded; >> >> I am wondering, is there a reason to make the solib parameter const for >> solib_unloaded but not for solib_loaded? >> >> Changing it to const seems to still compile just fine. If down the line >> an observer needs to modify the SO, the observer's signature can be >> adjusted. > > Just pragmatic reasons. Do you build with --enable-targets=all? > > It's because bsd_uthread_solib_loaded calls solib_read_symbols to read > in the symbols of the library it is looking for, and that requires a > non-const so_list. > Ah, indeed, my current build tree does not have --enable-targets=all, thanks. Having a user requiring a non-const ref is a good reason. Thanks, Lancelot.