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 5225F3856278 for ; Tue, 10 May 2022 12:58:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5225F3856278 Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-141-TVvKmG6zN0K5D1XQClVrGQ-1; Tue, 10 May 2022 08:58:53 -0400 X-MC-Unique: TVvKmG6zN0K5D1XQClVrGQ-1 Received: by mail-qk1-f197.google.com with SMTP id 63-20020a370c42000000b006a063777620so4057663qkm.21 for ; Tue, 10 May 2022 05:58:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=DcSuOKC9L25NoTi20m2sBTogo7HHBT7wSNN3M9gty8Y=; b=Ns72Q1Rp9eoprmdzmreBJ4Z5OozDYtpibzxtC4069/L0avf0JK9TlbULjzfXYUwPJe zNxExCy+nGo40+w6dCaIGKcJeSq35szmwintHqOgLaqQj+m0BVg4pxelLXDGITR9BHYX Ka/mE9GWyjRyXHQvokroYNAmoMF+HUBJ6fYA122bg4Is6KaR2iuP5x7H22Hpjrp5aRhX P+xACrcGEQEgCRvPKGsmOnyah0Ru8yk59gnPFEYwck96vKcOlVkq8SP2elUErRAvIiD3 nBB9EXJk3DnfBWMFqwovWbNYTkfodR5sVh0rggsY4vJUb+IPehmHnmw2Q067klveDdhU XqKg== X-Gm-Message-State: AOAM532R5pCOSZhhbo5lpATn/1TXgTpg6axRYt/rMNKWBiLEZEfkNECf ec4mUNuhZAz4LT0PkbVYiXilhVg/zqz0MrXZIxcY0b+hnPFtWnVXRMYsAoSN+e76masKjJN4pHf Cfb63jHXZLqsgxv5d3w== X-Received: by 2002:a05:620a:19a7:b0:6a0:2f0:c80c with SMTP id bm39-20020a05620a19a700b006a002f0c80cmr15078121qkb.209.1652187531945; Tue, 10 May 2022 05:58:51 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx5S51Q2bwPc/MYz4Gn1qNrFe9tlfsP584V6fYyTNyHRnZMIofXn2vXjvoCGyiKiIapHldbpw== X-Received: by 2002:a05:620a:19a7:b0:6a0:2f0:c80c with SMTP id bm39-20020a05620a19a700b006a002f0c80cmr15078102qkb.209.1652187531550; Tue, 10 May 2022 05:58:51 -0700 (PDT) Received: from [192.168.1.100] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id 191-20020a3705c8000000b0069fc13ce204sm8665053qkf.53.2022.05.10.05.58.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 10 May 2022 05:58:48 -0700 (PDT) Message-ID: <580b12c2-fa83-d268-373d-64bee758bf85@redhat.com> Date: Tue, 10 May 2022 08:58:46 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v2] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497] To: Marek Polacek , GCC Patches , Joseph Myers References: <20220507221456.552767-1-polacek@redhat.com> From: Jason Merrill In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-25.7 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_STOCKGEN, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 May 2022 12:58:56 -0000 On 5/7/22 18:26, Marek Polacek wrote: > Corrected version that avoids an uninitialized warning: > > This PR complains that we emit the "enumeration value not handled in > switch" warning even though the enumerator was marked with the > [[maybe_unused]] attribute. > > The first snag was that I couldn't just check TREE_USED, because > the enumerator could have been used earlier in the function, which > doesn't play well with the c_do_switch_warnings warning. Instead, > I had to check the attributes on the CONST_DECL directly, which led > to the second, and worse, snag: in C we don't have direct access to > the CONST_DECL for the enumerator. I wonder if you want to change that instead of working around it? > I had to handle that by adding > a new function that extracts the decl from an identifier's binding. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > PR c++/105497 > > gcc/c-family/ChangeLog: > > * c-common.h (get_decl_for_identifier): Declare. > * c-warn.cc (c_do_switch_warnings): Don't warn about unhandled > enumerator when it was marked as unused. > > gcc/c/ChangeLog: > > * c-decl.cc (get_decl_for_identifier): New. > > gcc/cp/ChangeLog: > > * tree.cc (get_decl_for_identifier): New. > > gcc/testsuite/ChangeLog: > > * c-c++-common/Wswitch-1.c: New test. > * g++.dg/warn/Wswitch-4.C: New test. > --- > gcc/c-family/c-common.h | 1 + > gcc/c-family/c-warn.cc | 24 ++++++++++-- > gcc/c/c-decl.cc | 10 +++++ > gcc/cp/tree.cc | 8 ++++ > gcc/testsuite/c-c++-common/Wswitch-1.c | 29 ++++++++++++++ > gcc/testsuite/g++.dg/warn/Wswitch-4.C | 52 ++++++++++++++++++++++++++ > 6 files changed, 121 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/Wswitch-1.c > create mode 100644 gcc/testsuite/g++.dg/warn/Wswitch-4.C > > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h > index f10be9bd67e..c4221089a18 100644 > --- a/gcc/c-family/c-common.h > +++ b/gcc/c-family/c-common.h > @@ -831,6 +831,7 @@ extern tree (*make_fname_decl) (location_t, tree, int); > > /* In c-decl.cc and cp/tree.cc. FIXME. */ > extern void c_register_addr_space (const char *str, addr_space_t as); > +extern tree get_decl_for_identifier (tree); > > /* In c-common.cc. */ > extern bool in_late_binary_op; > diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc > index cae89294aea..03cbc0541b2 100644 > --- a/gcc/c-family/c-warn.cc > +++ b/gcc/c-family/c-warn.cc > @@ -1738,8 +1738,23 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location, > for (chain = TYPE_VALUES (type); chain; chain = TREE_CHAIN (chain)) > { > tree value = TREE_VALUE (chain); > + tree id = TREE_PURPOSE (chain); > + tree attrs = NULL_TREE; > + /* In C++, the TREE_VALUE is a CONST_DECL. */ > if (TREE_CODE (value) == CONST_DECL) > - value = DECL_INITIAL (value); > + { > + attrs = DECL_ATTRIBUTES (value); > + value = DECL_INITIAL (value); > + } > + /* In C, the TREE_VALUE is an integer constant. The TREE_PURPOSE is > + an identifier from which we must tease out the CONST_DECL. */ > + else if (tree decl = get_decl_for_identifier (id)) > + attrs = DECL_ATTRIBUTES (decl); > + /* Track if the enumerator was marked as unused. We can't use > + TREE_USED here: it could have been set on the enumerator if > + the enumerator was used earlier. */ > + const bool unused_p = (lookup_attribute ("unused", attrs) > + || lookup_attribute ("maybe_unused", attrs)); Why is this calculation... > node = splay_tree_lookup (cases, (splay_tree_key) value); > if (node) > { > @@ -1769,6 +1784,10 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location, > /* We've now determined that this enumerated literal isn't > handled by the case labels of the switch statement. */ > > + /* Don't warn if the enumerator was marked as unused. */ > + if (unused_p) > + continue; ...separate from this test? > /* If the switch expression is a constant, we only really care > about whether that constant is handled by the switch. */ > if (cond && tree_int_cst_compare (cond, value)) > @@ -1791,8 +1810,7 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location, > (default_node || !warn_switch > ? OPT_Wswitch_enum > : OPT_Wswitch), > - "enumeration value %qE not handled in switch", > - TREE_PURPOSE (chain)); > + "enumeration value %qE not handled in switch", id); > } > > /* Warn if there are case expressions that don't correspond to > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc > index c701f07befe..c2c813d922a 100644 > --- a/gcc/c/c-decl.cc > +++ b/gcc/c/c-decl.cc > @@ -12466,4 +12466,14 @@ c_check_in_current_scope (tree decl) > return b != NULL && B_IN_CURRENT_SCOPE (b); > } > > +/* Returns the symbol associated with an identifier ID. Currently, this > + is used only in c_do_switch_warnings so that we can obtain the CONST_DECL > + associated with an enumerator identifier. */ > + > +tree > +get_decl_for_identifier (tree id) > +{ > + return I_SYMBOL_DECL (id); > +} > + > #include "gt-c-c-decl.h" > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc > index 979728027ed..13852dc6446 100644 > --- a/gcc/cp/tree.cc > +++ b/gcc/cp/tree.cc > @@ -6138,6 +6138,14 @@ maybe_adjust_arg_pos_for_attribute (const_tree fndecl) > return n > 0 ? n - 1 : 0; > } > > +/* Stub for c-common. Currently this has no use in the C++ front end. */ > + > +tree > +get_decl_for_identifier (tree) > +{ > + gcc_unreachable (); > +} > + > > /* Release memory we no longer need after parsing. */ > void > diff --git a/gcc/testsuite/c-c++-common/Wswitch-1.c b/gcc/testsuite/c-c++-common/Wswitch-1.c > new file mode 100644 > index 00000000000..de9ee03b0a3 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/Wswitch-1.c > @@ -0,0 +1,29 @@ > +/* PR c++/105497 */ > +/* { dg-options "-Wswitch" } */ > + > +enum E { > + A, > + B, > + C __attribute((unused)), > + D > +}; > + > +void > +g (enum E e) > +{ > + switch (e) > + { > + case A: > + case B: > + case D: > + break; > + } > + > + switch (e) // { dg-warning "not handled in switch" } > + { > + case A: > + case B: > + case C: > + break; > + } > +} > diff --git a/gcc/testsuite/g++.dg/warn/Wswitch-4.C b/gcc/testsuite/g++.dg/warn/Wswitch-4.C > new file mode 100644 > index 00000000000..553a57d777b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wswitch-4.C > @@ -0,0 +1,52 @@ > +// PR c++/105497 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wswitch" } > + > +enum class Button > +{ > + Left, > + Right, > + Middle, > + NumberOfButtons [[maybe_unused]] > +}; > + > +enum class Sound > +{ > + Bark, > + Meow, > + Hiss, > + Moo __attribute((unused)) > +}; > + > +enum class Chordata > +{ > + Urochordata, > + Cephalochordata, > + Vertebrata > +}; > + > +int main() > +{ > + Button b = Button::Left; > + switch (b) { // { dg-bogus "not handled" } > + case Button::Left: > + case Button::Right: > + case Button::Middle: > + break; > + } > + > + Sound s = Sound::Bark; > + switch (s) { // { dg-bogus "not handled" } > + case Sound::Bark: > + case Sound::Meow: > + case Sound::Hiss: > + break; > + } > + > + Chordata c = Chordata::Vertebrata; > + switch (c) { // { dg-warning "not handled" } > + case Chordata::Cephalochordata: > + case Chordata::Vertebrata: > + break; > + } > +} > > base-commit: 0c723bb4be2a67657828b692997855afcdc5d286