From patchwork Thu Feb 23 17:35:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 65529 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 AF86E385B516 for ; Thu, 23 Feb 2023 17:36:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AF86E385B516 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677173774; bh=seDinaEL0rZXmbcd7j8BDwcQ4o1W1G83SwgrcGagBEU=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=mTBJ2vnS98m4DbyzvP/0QNbY+tobtPNT2tmcCVsXVlt/varVXNM26wFVR81TaeZZp KKMmwQ5QxoifwrFmTfNJtJ5wNyvRu15xxSG59iBfirvtexFW7prAfw9DV1GRN/uZGf m75/3ony+DLT3VA+I92bbEoi38zT4DJpaYlMCLI8= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id AADC83858C5E for ; Thu, 23 Feb 2023 17:35:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AADC83858C5E Received: from smarchi-efficios.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 147BD1E128; Thu, 23 Feb 2023 12:35:43 -0500 (EST) To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 1/2] gdbsupport: ignore -Wenum-constexpr-conversion in enum-flags.h Date: Thu, 23 Feb 2023 12:35:40 -0500 Message-Id: <20230223173541.51256-1-simon.marchi@efficios.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 X-Spam-Status: No, score=-3497.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_DMARC_NONE, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_SOFTFAIL, 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-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" When building with clang 16, we get: CXX gdb.o In file included from /home/smarchi/src/binutils-gdb/gdb/gdb.c:19: In file included from /home/smarchi/src/binutils-gdb/gdb/defs.h:65: /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/enum-flags.h:95:52: error: integer value -1 is outside the valid range of values [0, 15] for this enumeration type [-Wenum-constexpr-conversion] integer_for_size(T (-1) < T (0))>::type ^ The error message does not make it clear in the context of which enum flag this fails (i.e. what is T in this context), but it doesn't really matter, we have similar warning/errors for many of them, if we let the build go through. clang is right that the value -1 is invalid for the enum type we cast -1 to. However, we do need this expression in order to select an integer type with the appropriate signedness. That is, with the same signedness as the underlying type of the enum. I first wondered if that was really needed, if we couldn't use std::underlying_type for that. It turns out that the comment just above says: /* Note that std::underlying_type is not what we want here, since that returns unsigned int even when the enum decays to signed int. */ I was surprised, because std::is_signed> returns the right thing. So I tried replacing all this with std::underlying_type, see if that would work. Doing so causes some build failures in unittests/enum-flags-selftests.c: CXX unittests/enum-flags-selftests.o /home/smarchi/src/binutils-gdb/gdb/unittests/enum-flags-selftests.c:254:1: error: static assertion failed due to requirement 'gdb::is_same, selftests::enum_flags_tests::RE, enum_flags, selftests::enum_flags_tests::RE2, enum_flags, selftests::enum_fla gs_tests::URE, int>, selftests::enum_flags_tests::check_valid_expr254::archetype, selftests::enum_flags_tests::RE, enum_flags, selfte sts::enum_flags_tests::RE2, enum_flags, selftests::enum_flags_tests::URE, unsigned int>>::value == true': CHECK_VALID (true, int, true ? EF () : EF2 ()) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/smarchi/src/binutils-gdb/gdb/unittests/enum-flags-selftests.c:91:3: note: expanded from macro 'CHECK_VALID' CHECK_VALID_EXPR_6 (EF, RE, EF2, RE2, UEF, URE, VALID, EXPR_TYPE, EXPR) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/valid-expr.h:105:3: note: expanded from macro 'CHECK_VALID_EXPR_6' CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1, typename T2, \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/valid-expr.h:66:3: note: expanded from macro 'CHECK_VALID_EXPR_INT' static_assert (gdb::is_detected_exact, \ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This is a bit hard to decode, but basically enumerations have the following funny property that they decay into a signed int, even if their implicit underlying type is unsigned. This code: enum A {}; enum B {}; int main() { std::cout << std::is_signed::type>::value << std::endl; std::cout << std::is_signed::type>::value << std::endl; auto result = true ? A() : B(); std::cout << std::is_signed::value << std::endl; } produces: 0 0 1 So, the "CHECK_VALID" above checks that this property works for enum flags the same way as it would if you were using their underlying enum types. And somehow, changing integer_for_size to use std::underlying_type breaks that. Since the current code does what we want, and I don't see any way of doing it differently, ignore -Wenum-constexpr-conversion around it. Change-Id: Ibc82ae7bbdb812102ae3f1dd099fc859dc6f3cc2 --- gdbsupport/enum-flags.h | 3 +++ include/diagnostics.h | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h index 700037f61260..41ac7838f060 100644 --- a/gdbsupport/enum-flags.h +++ b/gdbsupport/enum-flags.h @@ -91,9 +91,12 @@ template<> struct integer_for_size<8, 1> { typedef int64_t type; }; template struct enum_underlying_type { + DIAGNOSTIC_PUSH + DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION typedef typename integer_for_size(T (-1) < T (0))>::type type; + DIAGNOSTIC_POP }; namespace enum_flags_detail diff --git a/include/diagnostics.h b/include/diagnostics.h index d3ff27bc0080..41e6db65391f 100644 --- a/include/diagnostics.h +++ b/include/diagnostics.h @@ -76,6 +76,11 @@ # define DIAGNOSTIC_ERROR_SWITCH \ DIAGNOSTIC_ERROR ("-Wswitch") +# if __has_warning ("-Wenum-constexpr-conversion") +# define DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION \ + DIAGNOSTIC_IGNORE ("-Wenum-constexpr-conversion") +# endif + #elif defined (__GNUC__) /* GCC */ # define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS \ @@ -155,4 +160,8 @@ # define DIAGNOSTIC_ERROR_SWITCH #endif +#ifndef DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION +# define DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION +#endif + #endif /* DIAGNOSTICS_H */