From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 8FC9E3885528 for ; Fri, 9 Dec 2022 19:27:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8FC9E3885528 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1670614033; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=cSdkOEAwsO74Xvz25wQwRV4HJCM+UVqdkMiyu8YdgRA=; b=TNtQrCQuVDDu8x613XlRBrDpdWSP7y2Pnz0qAp4KKDXoj52zLd7fp3n8y66h83kwFtrdSB oV/vP8ao51F/DyBuJsnXI01G///lTS54hsN8+CxXVlAa2+og4s6G+3AsRCmA3pOg2mWiWT DKq1bk94ia/mbfeAq05uuajZmBFu2Y4= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-213-Q7-wC9GGPq6vVvfYYoDrog-1; Fri, 09 Dec 2022 14:27:11 -0500 X-MC-Unique: Q7-wC9GGPq6vVvfYYoDrog-1 Received: by mail-wm1-f72.google.com with SMTP id x10-20020a05600c420a00b003cfa33f2e7cso309567wmh.2 for ; Fri, 09 Dec 2022 11:27:11 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=cSdkOEAwsO74Xvz25wQwRV4HJCM+UVqdkMiyu8YdgRA=; b=xHsKpglMiOintGhIa4XRaF0eKc7d47rk3qCf7KyM4g5zJNpblw8F8bNoJfterOxTTi e1nud2fBcSnFrwnNEgIr7LE3Dj4a60YAH7gbvgiGTu/WAwFQkQ124dUIDZqzimT5Ek49 6osMR9DCnlmJlh2T4LCYDApBbKMMAEUEh1Sl7LWei3xzlVmSZj+ZAJeUUhVtWbkU1ZpN dmZXuu2GWy97fiKscMZu8mmVp8PF0L2idtwnxfcEpGsD7GweSjHz7335vLLbXD65iUM2 c4/seEcChi2Zg2v+caoD4hs5f9F2HBq8ExaZk1wohpbsWDFHi8JKU0K7OAxhBhy4WQYt q+fw== X-Gm-Message-State: ANoB5pmMvviBD45E3HDflzW4tkL78/dqwxkHOWv+2hemYFlLu5k1TSFM 3mXUILqWBq0Nc1kNOQ7ZqxpVtvUThhL9W1vz7xf6ElMG7zEbc1gv1OIv0R8r+JshKhQOnHGadwG BHiuWtIhyH3bSiwPH7sR1ZA== X-Received: by 2002:adf:fac5:0:b0:242:2bf4:d6b3 with SMTP id a5-20020adffac5000000b002422bf4d6b3mr4279901wrs.40.1670614030665; Fri, 09 Dec 2022 11:27:10 -0800 (PST) X-Google-Smtp-Source: AA0mqf6/+RYYuX5yZmGU2MHVM0CxI5YJJo1Wt/43HZEjCYB+UVJo2LA0ZY7RkTNRpTu5qzeZ2LnAhg== X-Received: by 2002:adf:fac5:0:b0:242:2bf4:d6b3 with SMTP id a5-20020adffac5000000b002422bf4d6b3mr4279895wrs.40.1670614030502; Fri, 09 Dec 2022 11:27:10 -0800 (PST) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id e6-20020adffc46000000b002425c6d30c6sm2354597wrs.117.2022.12.09.11.27.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Dec 2022 11:27:10 -0800 (PST) From: Andrew Burgess To: Simon Marchi , Tom Tromey , Andrew Burgess via Gdb-patches Subject: Re: [PATCH 2/2] gdb: add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT In-Reply-To: <77bf3650-0b63-db6e-3bff-8d131c6a43e5@simark.ca> References: <73bdc6db114e7a511a2b91d9a9f5ac6f9f589881.1670348436.git.aburgess@redhat.com> <87y1rgh3sg.fsf@tromey.com> <77bf3650-0b63-db6e-3bff-8d131c6a43e5@simark.ca> Date: Fri, 09 Dec 2022 19:27:09 +0000 Message-ID: <87359o2x9u.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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: Simon Marchi writes: > 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; > } Thanks for the feedback. I did originally have a free function here, but wanted to check the code worked fine with a lambda too ... and it just got left like that. I'll switch it back to a free function before committing. Thanks, Andrew > > 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