From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 8447E3884C9E for ; Fri, 9 Dec 2022 17:48:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8447E3884C9E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [132.207.214.75] (Sansfil-Eduroam-Externe-214-75.polymtl.ca [132.207.214.75]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id EA04D1E0D3; Fri, 9 Dec 2022 12:48:39 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1670608120; bh=oCeJFOQjyIK7R+Q+hmHJh+h5GW2Ed63KpdTmehCXG1s=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=Mk12vkmullM5ra5r60LVg89XhPdCnap0oXEpoO2KPm6OV9hxIML56BFxY1sD9SowV PMDpdRUo+zZglUp7Be0+s/gEK7FR+q6GynSvv788UHs/c+fxmgat7R/fcxn6PMTdfn kJugLsqsMu9414xnhIWpfjOJXtfCliTWTDPoqRc4= Message-ID: <77bf3650-0b63-db6e-3bff-8d131c6a43e5@simark.ca> Date: Fri, 9 Dec 2022 12:48:39 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH 2/2] gdb: add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT Content-Language: fr To: Tom Tromey , Andrew Burgess via Gdb-patches Cc: Andrew Burgess References: <73bdc6db114e7a511a2b91d9a9f5ac6f9f589881.1670348436.git.aburgess@redhat.com> <87y1rgh3sg.fsf@tromey.com> From: Simon Marchi In-Reply-To: <87y1rgh3sg.fsf@tromey.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_STOCKGEN,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 12/9/22 12:42, Tom Tromey wrote: >>>>>> "Andrew" == Andrew Burgess via Gdb-patches writes: > > Andrew> After the previous commit converted symbol lookup debug to use the new > Andrew> debug scheme, this commit adds SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT. > > Andrew> The previous commit updated 'set debug symbol-lookup' to use the new > Andrew> debug scheme, however SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT still does > Andrew> not exist. The reason for this is that 'set debug symbol-lookup' > Andrew> controls an integer variable, rather than a bool, and, depending on > Andrew> the value of this variable, different amounts of debug will be > Andrew> produced. > > Andrew> Currently the *_SCOPED_DEBUG_ENTER_EXIT mechanism will only work for > Andrew> control variables of type bool, this is because the underlying > Andrew> scoped_debug_start_end class can only handle variables of type bool. > > Andrew> This commit templates scoped_debug_start_end so that the class can > Andrew> accept either a 'bool &' or an invokable object, e.g. a lambda > Andrew> function, or a function pointer. > > Andrew> The existing scoped_debug_start_end and scoped_debug_enter_exit macros > Andrew> in common-debug.h are updated to support scoped_debug_enter_exit being > Andrew> templated, however, nothing outside of common-debug.h needs to change. > > Andrew> I've then added SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT in symtab.h, and > Andrew> added a couple of token uses in symtab.c. I didn't want to add too > Andrew> much in this first commit, this is really about updating > Andrew> common-debug.h to support this new ability. > > This seems fine. I suppose inlining will make it not cost too much... ? > > Andrew> +#define SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT \ > Andrew> + scoped_debug_enter_exit ([&] () -> bool { \ > Andrew> + return symbol_lookup_debug > 0; \ > Andrew> + }, "symbol-lookup") > > I wonder whether the [&] is needed here. I couldn't see what it is > capturing. Same comment here. Instead of a lambda, you could have single function: static bool symbol_lookup_debug_enabled () { return symbol_lookup_debug > 0; } And then you could use it in symbol_lookup_debug_printf too, to factor the logic. You could have symbol_lookup_debug_v_enabled too for symmetry if you wanted. Otherwise, it all LGTM: Approved-By: Simon Marchi Simon