From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id aRI7NrJevmpGxiMAWB0awg (envelope-from ) for ; Thu, 01 Oct 2026 09:22:58 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=polymtl.ca header.i=@polymtl.ca header.a=rsa-sha256 header.s=oct2025 header.b=CeRBX6NH; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id D935E1E051; Thu, 01 Oct 2026 09:22:58 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 57B401E033 for ; Thu, 01 Oct 2026 09:22:58 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BA3284B99F5B for ; Thu, 1 Oct 2026 13:22:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BA3284B99F5B Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=polymtl.ca header.i=@polymtl.ca header.a=rsa-sha256 header.s=oct2025 header.b=CeRBX6NH Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id AF7F74BAE7D8 for ; Thu, 1 Oct 2026 13:22:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AF7F74BAE7D8 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org AF7F74BAE7D8 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=132.207.4.11 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1790860951; cv=none; b=o5lYpKKqu1N9VwzGVRhaoClozXL46xjNKmLchI/DtpfGc3bKycMX3n+Ays8GLBCsXc8a9yZZ0TtJ1a6c6QR7c3PKJYGXFOSms5J2Cqea2SxOkHPOREWOy+sxARwpQ8cyLOY+hVcg/LeB2OjupQK6jZ2hQhWHCbXfjbVHG+8LzAI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1790860951; c=relaxed/simple; bh=mlAWDTP5uY9XPSkc4BGbe+7tvbCQcI+WOxjS5OOB28M=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=hFa9uvkPF44Ueh64xq1nbgS0rKvnJs4m+AGJVs85pefmiIpe6LnO8sZSrOqTGUdvRh14cQjRAILg5kA80CQ4agc9Aw4a995/xhD3QQecbZdATaWTsVgpWDkP7kaTiRBXs58DwRt1cF4JRNR2wR3hH3Hvpdc5uzw5gNg+hWy9e1Y= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=polymtl.ca header.i=@polymtl.ca header.a=rsa-sha256 header.s=oct2025 header.b=CeRBX6NH DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AF7F74BAE7D8 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 691DMDCZ103621 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 1 Oct 2026 09:22:19 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 691DMDCZ103621 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=oct2025; t=1790860939; bh=DBVJDLWQUm1D3hyt30bHYLhuhrMEyPCaEgNOEbOw2F8=; h=Date:Subject:To:Cc:From:In-Reply-To:From; b=CeRBX6NHErk3yeLr+g4j/nXuLhxht9q21p7YSAb1eHS5hdQ9lLix+ONP1tyVMbYRJ vIC7vUGr76F7yVtxVsUHJbSOcD1CJuUAvaCL+U6TUK2YRGymLG9lKj+t7Bk4/W1Nj6 wDCpRn7gCDdrnB8Mfyeo4Y81TtCKEpNsjAY5ElYDYMXGJRmZ3VC6sxVOBv0oozVby0 4rbNq0IZowaFXOifHs4p4NynKPtOQCSZ6uF7vYXsDrrvbcG7QckEa0PZGEBtQnSAbC 57dobkgeMlnqsYTx/Rl3WS58yWeuGmhqhd9QeVHplKUDuS2Du0qcNypKMZEdvJ0C6C e7P/SINXPzmEQ== Received: by simark.ca (Postfix) id B4A6E1E033; Thu, 01 Oct 2026 09:22:11 -0400 (EDT) Message-ID: <15465f38-b454-4e6e-9c32-625f6262d6df@polymtl.ca> Date: Thu, 1 Oct 2026 09:22:11 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 01/11] gdb: add symbol_search_kind enum for global_symbol_searcher To: Andrew Burgess , gdb-patches@sourceware.org Cc: Simon Marchi References: <20260929194119.155169-1-simon.marchi@efficios.com> <20261001020952.698419-2-simon.marchi@polymtl.ca> <87cxttzp9p.fsf@redhat.com> Content-Language: fr From: Simon Marchi In-Reply-To: <87cxttzp9p.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 1 Oct 2026 13:22:14 +0000 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 10/1/26 6:30 AM, Andrew Burgess wrote: > > I think this is looking good. Just one minor comment inline below: > > simon.marchi@polymtl.ca writes: > >> @@ -5166,20 +5179,18 @@ symtab_symbol_info (bool quiet, bool exclude_minsyms, >> const char *classname; >> switch (kind) >> { >> - case VAR_DOMAIN: >> + case symbol_search_kind::VARIABLE: >> classname = "variable"; >> break; >> - case FUNCTION_DOMAIN: >> + case symbol_search_kind::FUNCTION: >> classname = "function"; >> break; >> - case TYPE_DOMAIN: >> + case symbol_search_kind::TYPE: >> classname = "type"; >> break; >> - case MODULE_DOMAIN: >> + case symbol_search_kind::MODULE: >> classname = "module"; >> break; >> - default: >> - gdb_assert_not_reached ("invalid domain enum"); >> } > > I initially wrote: please don't drop the default case, but then I > checked the V1 review and saw this change was made specifically to > address the feedback there. > > What I don't understand is you said: > >>> I think we could drop the "default" case as part of this refactoring. All >>> cases are handled; we lose compile time warnings for unhandled cases with >>> the "default" case. >> >> Agreed that we can drop the default. Note that we don't build with >> -Wswitch, but we specifically enable it locally here and there. > > If the warning is mostly off, including here, then deleting the default > seems like the wrong choice. > > I'm not too fussed, I think there's a middle group, that would keep my > happy: > > 1. initialise classname to nullptr, and > > 2. follow this switch statement with: > gdb_assert (classname != nullptr); You're right, I applied the suggestion without thinking too much about it. We definitely want an assert to fire instead of printing from an uninitialized pointer. If that same code was in a function of its own, I would have done it like this: static const char * ssk_to_string (symbol_search_kind kind) { switch (kind) { case symbol_search_kind::VARIABLE: return "variable"; case symbol_search_kind::FUNCTION: return "function"; case symbol_search_kind::TYPE: return "type"; case symbol_search_kind::MODULE: return "module"; } gdb_assert_not_reached ("invalid symbol_search_kind value"); } There is even an example of that in the same patch (the to_search_flags function). I'll initialize the variable with nullptr and add a nullptr check after the switch. I prefer this over the default, because if we ever enable -Wswitch (which will probably never happen, I tried it before and it was painful), then we'll get a compile-time error if we are missing a case. I don't think that this code is duplicated anywhere, so I don't think it's worth moving it to a separate function. > > Even with the warning on it doesn't protect us from random garbage being > cast to type symbol_search_kind and then passed to this function, which > the 'default:' case does protect against. But the trailing assert is > probably better anyway as it ensures each switch case is doing what > needs to be done. > > With that: > > Approved-By: Andrew Burgess Thanks, Simon