From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x636.google.com (mail-ej1-x636.google.com [IPv6:2a00:1450:4864:20::636]) by sourceware.org (Postfix) with ESMTPS id 42CAE384D158 for ; Thu, 20 Oct 2022 17:58:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 42CAE384D158 Received: by mail-ej1-x636.google.com with SMTP id sc25so1283246ejc.12 for ; Thu, 20 Oct 2022 10:58:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cbalyuy8gFPWaMZtEOwreax068JV8pW55MPcreM1F6g=; b=ojLXn4DA/YKtRHcV3ERSL50TgMWksLNghFp/RLk0QcT8+OX2LCPgaDZH46EeR+CPKJ XoJ9fWMt6mZ4YyEJ3I6ku9DCghV810ZDg/1T7v/ds8doCxF2CTxhi8hBcgTIhOQP4Qxl 9W9NomhdJLmpRFlus+P384Nd951WLMiSVjInRckrW+HsFOgsB4A1UgfzN1z+ogthmLMl qL+yhkog6uj6/K/1VUCj+5LD0iJmym3N6Owoe5G8Zxcrz33ijeGhHfzpSr8alwPt9X2X jLlrtX4R/PGoxTvmzgAtMNwlgtsNagzNkeZj/5ayKOEQVKCKq3WLPJDdYjli0w/XRsNO hpIw== X-Gm-Message-State: ACrzQf3JhmN7SfHvZO8tNPXhtVjbRp6Lec2WRscp5p9Vlh3glh9Tc1dN 0UWFqP74E0MJfNeYpj2ZLpu0JVLVCgfwru1/BhlXpg== X-Google-Smtp-Source: AMsMyM5dQlhtTr1zYKEx3LlHXO7EnkLEtkVij0J9GkfzSDbw2nBlpoP+HA/LLJDvTZXLknrXK4rias4UEg45Cergz2k= X-Received: by 2002:a17:906:dc8f:b0:78d:f675:226 with SMTP id cs15-20020a170906dc8f00b0078df6750226mr12071496ejc.745.1666288705060; Thu, 20 Oct 2022 10:58:25 -0700 (PDT) MIME-Version: 1.0 References: <20220909080135.22921-1-grees@undo.io> <20220926124629.174047-1-grees@undo.io> <87fsftateu.fsf@redhat.com> In-Reply-To: <87fsftateu.fsf@redhat.com> From: Gareth Rees Date: Thu, 20 Oct 2022 18:58:13 +0100 Message-ID: Subject: Re: [PATCH v4] [PR mi/29554] New PRINT-VALUES option '--scalar-values'. To: Andrew Burgess Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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 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: , X-List-Received-Date: Thu, 20 Oct 2022 17:58:50 -0000 Thank you for the thorough review. I implemented all your suggestions, and have attempted to add tests covering all the behaviour of the new '--scalar-values' option. The revised patch is v5, posted separately. Andrew Burgess wrote: > > +/* Print VAL according to VALUES? */ > > + > > +static bool > > +print_value_p (struct value *val, enum print_values values) > > +{ > > + switch (values) > > + { > > + default: > > + case PRINT_NO_VALUES: > > + return false; > > + case PRINT_ALL_VALUES: > > + return true; > > + case PRINT_SIMPLE_VALUES: > > + return mi_simple_values_type_p (value_type (val)); > > + case PRINT_SCALAR_VALUES: > > + return val_print_scalar_type_p (value_type (val)); > > + } > > +} > > I think you should probably fold together this print_value_p and > print_symbol_p in mi-cmd-stack.c. Maybe create an mi_print_type_p > somewhere, which can contain the core switch table? I have implemented this suggestion. Note that a consequence is that in the PRINT_NO_VALUES and PRINT_ALL_VALUES cases, we now incur a call to value_type(val) or sym->type() when previously we just returned false or true respectively. I think this is probably worth it, in order to reduce the amount of duplicated code, but just want to highlight the small change in behaviour here.