From patchwork Mon Nov 7 17:26:04 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 60137 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1E95E3858422 for ; Mon, 7 Nov 2022 17:26:24 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by sourceware.org (Postfix) with ESMTPS id 2AF923858C00 for ; Mon, 7 Nov 2022 17:26:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2AF923858C00 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f50.google.com with SMTP id a14so17295106wru.5 for ; Mon, 07 Nov 2022 09:26:08 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=jDpnNU0olkxDreobLuAafKBDGiOgBqqusXeGnLAl7Aw=; b=QPTwjvmx+r3UKvwred0bUv1N4UIOROPzr8hqBx28ncwN4ICnjQohAdtXIwP85RUfCp 8wrmGeYv1P3ToIHwJJJRIN+HAhBTF4QueoVx9cft5pbSY6Tl6y/DcpcRofLCWufRi3vz O+gpU/+4qD6hVjAZM3weiIqZ2gwVAxFv4q/PsJp4+Fjlg7uk38QBeEhn+US5SlerVnU0 9YlIyYLkIesOS+dwD5MfcpifzoLsx74U2U2KjsuO9KDOlt6LS+uCp95/5I13yOTIxphD Tk565H11qE5EdCxusXE4r2rfkASm7z3b2wHqMvnQZYNiRvvyOnBH8zFWAuhJTsyfV4uh l0Bw== X-Gm-Message-State: ACrzQf1SaUt46CzfsVEUJXn4/ygG+d+K+1BDEQUhcCHApxwfl3Eo5Q4M qvq1+EjR0blnYPQZTxu60LZGR59FVok= X-Google-Smtp-Source: AMsMyM5CJiOPFChJfnCW1VwbtdQoTo0pwNwLqPkL98F48O9FSNtZ2E1goobgB3cPP/cuxXqewWCr9Q== X-Received: by 2002:a5d:68ce:0:b0:236:9981:d216 with SMTP id p14-20020a5d68ce000000b002369981d216mr31995670wrw.384.1667841966110; Mon, 07 Nov 2022 09:26:06 -0800 (PST) Received: from ?IPv6:2001:8a0:f93a:3b00:e038:5cdc:b8bf:4653? ([2001:8a0:f93a:3b00:e038:5cdc:b8bf:4653]) by smtp.gmail.com with ESMTPSA id m2-20020a056000008200b0023662d97130sm7836940wrx.20.2022.11.07.09.26.05 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 07 Nov 2022 09:26:05 -0800 (PST) Subject: [PATCH v5] enum_flags to_string (was: Re: [PATCH v2 07/29] Thread options & clone events (core + remote)) To: Simon Marchi , gdb-patches@sourceware.org References: <20220713222433.374898-1-pedro@palves.net> <20220713222433.374898-8-pedro@palves.net> <5d68cd36-e8d6-e8ad-5428-863e79742062@simark.ca> <6b91f8f6-d3b5-c44b-297c-ce1c3a1c80f6@palves.net> <6aee3c00-b334-97e2-62f7-0434822bcf7a@simark.ca> <47ef6c92-c4f6-448f-c4c3-71da4618107c@palves.net> <38cb6f92-34d5-4830-95d8-82bd0a2b113e@simark.ca> From: Pedro Alves Message-ID: Date: Mon, 7 Nov 2022 17:26:04 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <38cb6f92-34d5-4830-95d8-82bd0a2b113e@simark.ca> Content-Language: en-US X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" On 2022-10-31 12:47 p.m., Simon Marchi wrote: > On 10/28/22 14:23, Pedro Alves wrote: ... >> @@ -183,6 +194,52 @@ class enum_flags >> /* Binary operations involving some unrelated type (which would be a >> bug) are implemented as non-members, and deleted. */ >> >> + /* Convert this object to a std::string, using MAPPING as >> + enumerator-to-string mapping array. This is not meant to be >> + called directly. Instead, enum_flags specializations should have >> + their own to_string function wrapping this one, thus hidding the >> + mapping array from callers. */ >> + >> + template >> + std::string >> + to_string (const string_mapping (&mapping)[N]) const >> + { >> + enum_type flags = raw (); >> + std::string res = hex_string (flags); >> + res += " ["; >> + >> + bool need_space = false; >> + for (const auto &entry : mapping) >> + { >> + if ((flags & entry.flag) != 0) >> + { >> + /* Do op~ in the underlying type, because if enum_type's >> + underlying type is signed, op~ won't be defined for >> + it. */ >> + flags &= (enum_type) ~(underlying_type) entry.flag; > > This line gives me (GCC 12): > > CXX unittests/enum-flags-selftests.o > In file included from /home/simark/src/binutils-gdb/gdb/defs.h:65, > from /home/simark/src/binutils-gdb/gdb/unittests/enum-flags-selftests.c:20: > /home/simark/src/binutils-gdb/gdb/../gdbsupport/enum-flags.h: In instantiation of ‘std::string enum_flags::to_string(const string_mapping (&)[N]) const [with long unsigned int N = 2; E = selftests::enum_flags_tests::test_uflag; std::string = std::__cxx11::basic_string]’: > /home/simark/src/binutils-gdb/gdb/unittests/enum-flags-selftests.c:388:26: required from here > /home/simark/src/binutils-gdb/gdb/../gdbsupport/enum-flags.h:219:19: error: invalid conversion from ‘unsigned int’ to ‘enum_flags::enum_type’ {aka ‘selftests::enum_flags_tests::test_uflag’} [-fpermissive] > 219 | flags &= (enum_type) ~(underlying_type) entry.flag; > | ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | | > | unsigned int > > This builds though: > > flags = (enum_type) (flags & (enum_type) (~(underlying_type) entry.flag)); > > I'm not sure why, the operator & between an enum_type and an enum_type > is supposed to return an enum_type already. > Man... The problem is that enum_flags::to_string is defined inside the template class, before the global operators for enum_type are defined, so the to_string method is being compiled as if the global operators don't exist. I guess newer GCC is more stringent than the GCC 9.4 from Ubuntu 20.04, which is what I was using. The fix is then to move the method outside the class, after the operators. Here's a new version... I'm now also doing the op~ using an unsigned type, in case ubsan complains or something. And that made me realize, that only testing with the unsigned enum completely bypasses the need for these casts in the first place, so better re-add testing with the signed enum. Better yet, test both signed and unsigned. I also tweaked the testcase to no longer need "test_flag (0xff)" -- I remembered what was the original reason I had added that -- it was to check whether multiple leftover unmapped flags end up printed with a single hex. I also changed the numbers behind FLAG1, FLAG2, etc., to make it easier to come up with the numbers for the tests. Tested GCC 9, 13 (some self-built trunk version from a few months back), clang 10 and clang 13. -- >8 -- From 9ab8b34f50dd219d817d333d52a3d8430cff9db2 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 25 Oct 2022 15:39:37 +0100 Subject: [PATCH] enum_flags to_string This commit introduces shared infrastructure that can be used to implement enum_flags -> to_string functions. With this, if we want to support converting a given enum_flags specialization to string, we just need to implement a function that provides the enumerator->string mapping, like so: enum some_flag { SOME_FLAG1 = 1 << 0, SOME_FLAG2 = 1 << 1, SOME_FLAG3 = 1 << 2, }; DEF_ENUM_FLAGS_TYPE (some_flag, some_flags); static std::string to_string (some_flags flags) { static constexpr some_flags::string_mapping mapping[] = { MAP_ENUM_FLAG (SOME_FLAG1), MAP_ENUM_FLAG (SOME_FLAG2), MAP_ENUM_FLAG (SOME_FLAG3), }; return flags.to_string (mapping); } .. and then to_string(SOME_FLAG2 | SOME_FLAG3) produces a string like "0x6 [SOME_FLAG2 SOME_FLAG3]". If we happen to forget to update the mapping array when we introduce a new enumerator, then the string representation will pretty-print the flags it knows about, and then the leftover flags in hex (one single number). For example, if we had missed mapping SOME_FLAG2 above, we'd end up with: to_string(SOME_FLAG2 | SOME_FLAG3) => "0x6 [SOME_FLAG2 0x4]"); Other than in the unit tests included, no actual usage of the functionality is added in this commit. Change-Id: I835de43c33d13bc0c95132f42c3f97318b875779 --- gdb/unittests/enum-flags-selftests.c | 69 +++++++++++++++++++++++++--- gdbsupport/enum-flags.h | 66 ++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 6 deletions(-) base-commit: b7096df235486ce718c2a0bfda8a0db074dcb671 diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c index f52fc7220a1..0fd35262469 100644 --- a/gdb/unittests/enum-flags-selftests.c +++ b/gdb/unittests/enum-flags-selftests.c @@ -359,21 +359,47 @@ CHECK_VALID (true, bool, NF (1) == char (1)) enum test_flag { - FLAG1 = 1 << 1, - FLAG2 = 1 << 2, - FLAG3 = 1 << 3, + FLAG1 = 1 << 0, + FLAG2 = 1 << 1, + FLAG3 = 1 << 2, + FLAG4 = 1 << 3, }; enum test_uflag : unsigned { - UFLAG1 = 1 << 1, - UFLAG2 = 1 << 2, - UFLAG3 = 1 << 3, + UFLAG1 = 1 << 0, + UFLAG2 = 1 << 1, + UFLAG3 = 1 << 2, + UFLAG4 = 1 << 3, }; DEF_ENUM_FLAGS_TYPE (test_flag, test_flags); DEF_ENUM_FLAGS_TYPE (test_uflag, test_uflags); +/* to_string enumerator->string mapping functions used to test + enum_flags::to_string. These intentionally miss mapping a couple + enumerators each (xFLAG2, xFLAG4). */ + +static std::string +to_string_flags (test_flags flags) +{ + static constexpr test_flags::string_mapping mapping[] = { + MAP_ENUM_FLAG (FLAG1), + MAP_ENUM_FLAG (FLAG3), + }; + return flags.to_string (mapping); +} + +static std::string +to_string_uflags (test_uflags flags) +{ + static constexpr test_uflags::string_mapping mapping[] = { + MAP_ENUM_FLAG (UFLAG1), + MAP_ENUM_FLAG (UFLAG3), + }; + return flags.to_string (mapping); +} + static void self_test () { @@ -581,6 +607,37 @@ self_test () SELF_CHECK (ok); } + + /* Check string conversion. */ + { + SELF_CHECK (to_string_uflags (0) + == "0x0 []"); + SELF_CHECK (to_string_uflags (UFLAG1) + == "0x1 [UFLAG1]"); + SELF_CHECK (to_string_uflags (UFLAG1 | UFLAG3) + == "0x5 [UFLAG1 UFLAG3]"); + SELF_CHECK (to_string_uflags (UFLAG1 | UFLAG2 | UFLAG3) + == "0x7 [UFLAG1 UFLAG3 0x2]"); + SELF_CHECK (to_string_uflags (UFLAG2) + == "0x2 [0x2]"); + /* Check that even with multiple unmapped flags, we only print one + unmapped hex number (0xa, in this case). */ + SELF_CHECK (to_string_uflags (UFLAG1 | UFLAG2 | UFLAG3 | UFLAG4) + == "0xf [UFLAG1 UFLAG3 0xa]"); + + SELF_CHECK (to_string_flags (0) + == "0x0 []"); + SELF_CHECK (to_string_flags (FLAG1) + == "0x1 [FLAG1]"); + SELF_CHECK (to_string_flags (FLAG1 | FLAG3) + == "0x5 [FLAG1 FLAG3]"); + SELF_CHECK (to_string_flags (FLAG1 | FLAG2 | FLAG3) + == "0x7 [FLAG1 FLAG3 0x2]"); + SELF_CHECK (to_string_flags (FLAG2) + == "0x2 [0x2]"); + SELF_CHECK (to_string_flags (FLAG1 | FLAG2 | FLAG3 | FLAG4) + == "0xf [FLAG1 FLAG3 0xa]"); + } } } /* namespace enum_flags_tests */ diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h index cd500f55ff3..a8f2bfd7a5c 100644 --- a/gdbsupport/enum-flags.h +++ b/gdbsupport/enum-flags.h @@ -130,6 +130,17 @@ class enum_flags typedef E enum_type; typedef typename enum_underlying_type::type underlying_type; + /* For to_string. Maps one enumerator of E to a string. */ + struct string_mapping + { + E flag; + const char *str; + }; + + /* Convenience for to_string implementations, to build a + string_mapping array. */ +#define MAP_ENUM_FLAG(ENUM_FLAG) { ENUM_FLAG, #ENUM_FLAG } + public: /* Allow default construction. */ constexpr enum_flags () @@ -183,6 +194,18 @@ class enum_flags /* Binary operations involving some unrelated type (which would be a bug) are implemented as non-members, and deleted. */ + /* Convert this object to a std::string, using MAPPING as + enumerator-to-string mapping array. This is not meant to be + called directly. Instead, enum_flags specializations should have + their own to_string function wrapping this one, thus hidding the + mapping array from callers. + + Note: this is defined outside the template class so it can use + the global operators for enum_type, which are only defined after + the template class. */ + template + std::string to_string (const string_mapping (&mapping)[N]) const; + private: /* Stored as enum_type because GDB knows to print the bit flags neatly if the enum values look like bit flags. */ @@ -424,4 +447,47 @@ void operator>> (const enum_flags &, const any_type &) = delete; #endif /* __cplusplus */ +template +template +std::string +enum_flags::to_string (const string_mapping (&mapping)[N]) const +{ + enum_type flags = raw (); + std::string res = hex_string (flags); + res += " ["; + + bool need_space = false; + for (const auto &entry : mapping) + { + if ((flags & entry.flag) != 0) + { + /* Work with an unsigned version of the underlying type, + because if enum_type's underlying type is signed, op~ + won't be defined for it, and, bitwise operatons on signed + types are implementation defined. */ + using uns = typename std::make_unsigned::type; + flags &= (enum_type) ~(uns) entry.flag; + + if (need_space) + res += " "; + res += entry.str; + + need_space = true; + } + } + + /* If there were flags not included in the mapping, print them as + a hex number. */ + if (flags != 0) + { + if (need_space) + res += " "; + res += hex_string (flags); + } + + res += "]"; + + return res; +} + #endif /* COMMON_ENUM_FLAGS_H */