From patchwork Thu Nov 30 00:05:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lewis Hyatt X-Patchwork-Id: 80995 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 CD3093858433 for ; Thu, 30 Nov 2023 00:06:10 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-oi1-x22d.google.com (mail-oi1-x22d.google.com [IPv6:2607:f8b0:4864:20::22d]) by sourceware.org (Postfix) with ESMTPS id 544663858D1E for ; Thu, 30 Nov 2023 00:05:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 544663858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 544663858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::22d ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701302753; cv=none; b=ahtDFWbo7BAFD5cSYrXDFDnM36uEOJws/wCBwfKA9RgH2J0XkyDoP+Qrf+7c/x6mkQihj8AUpj+Qwsyw7rd5Y2faNRK4UrXaBYbR30x7bzJbknbAhX1kCoBD/LJePiGJT+E4lhkvkQs7pXU3i4LPr7AaQJP4G4QeicOTzaa74aM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701302753; c=relaxed/simple; bh=jxXEhGdJQ8HEyrn6kJsUY328aPvb3T9cEsnYsAr1zp4=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=p6PM4lIi0cNZwkcq9JKvvNwSqrs9x1nJ6mCR72yof62ecFaZtSC7JGS62/OCJ8t50rS7CWl+4m6rbdXmBF8Y4274L9hrS4mVRVQ68zuGD5qxWYt/KdUqOeLM1WjTiEJaDTfLW9CZLvt/LArGLhw7hY1N6kuRj0OqORPvdEpZZs8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-oi1-x22d.google.com with SMTP id 5614622812f47-3b86f3cdca0so211111b6e.3 for ; Wed, 29 Nov 2023 16:05:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701302750; x=1701907550; darn=gcc.gnu.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=c1Qt3viBV+yCoHlq4FQL44CxtXqpHGetziu0HLqCYC8=; b=Ze0k1ssY1NlpwCxS2co3AbqkvFXL7MkcXQhKcouiijJIt6kC6JyRyfYFt4xxPAC/NW woG+lLsMepncw8PvcuIpxUWfK/XnEeZIBOsSjvAeVZ4VsALDCxURKlJHZ8DgTlpkH0iB yKva543IEEpOcZen3nb45LBhnH04c/RzkMCzs64oSuL8gEc91eWoqZBhU2ulW7kbsoEY qWIIWaad7u+GTcS9fNv/9uG9eOHW051SJcQirISY2PcZemEajU241oXqWBoyNODcIudr GvW9+76JSsjlUVL081q8IBiGsxcATK0TTv15em6h1PVs6cto8XT5nnFOEVrx1aeYN+zn /CMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701302750; x=1701907550; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=c1Qt3viBV+yCoHlq4FQL44CxtXqpHGetziu0HLqCYC8=; b=G0Fyd7WOV7xFHO5y82oEdD6kucfGgM+E4Cg0ibqiwtBl3bgt97BvHMu2hUxItmO7Ri Fxb3cWBbjUxIHIRhGx1J8WmtDmpgEQWK5Qkt+KARilRzSkCxvkWGoGGNAZFbj0meEUMY lSKnMyncJG+bneyxRKZtEP7hoyRejdP/CbDeyZ5qR5HG0YJdSRC5S8I/iUBFLoyFylIo EdGuWkH+9RzV6A/d8M0BD7j/rIx3gOz6sU9sCMRKABrpRU0h0WN7OnGLUqJ2+PZVp73Y Txz4yhB/YIKXQ3/GKW9upDOOJSctRmF3OqfbYFbiaH8OJv4X0QwaX3F+q+gtPBiI+74t kIyg== X-Gm-Message-State: AOJu0YwUzd+Dlu3q7iTBKP9iXq/p2SHWsPiUuwqkG3sl+yuftaa347UX 39hYoAieZHIAi7cLVvILGqi75Mvhmc4= X-Google-Smtp-Source: AGHT+IHEizXk7BipqvRp6Jk8opsH/SLzQ7WBd0eNxgPEpIJ/s+jqAII3Faj7iwdoHWlggEs7uZrqEQ== X-Received: by 2002:a05:6358:260e:b0:16e:58:908b with SMTP id l14-20020a056358260e00b0016e0058908bmr19845883rwc.21.1701302750388; Wed, 29 Nov 2023 16:05:50 -0800 (PST) Received: from ldh-imac.local (96-67-140-173-static.hfc.comcastbusiness.net. [96.67.140.173]) by smtp.gmail.com with ESMTPSA id kr6-20020ac861c6000000b004180fb5c6adsm6047597qtb.25.2023.11.29.16.05.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Nov 2023 16:05:49 -0800 (PST) Date: Wed, 29 Nov 2023 19:05:47 -0500 From: Lewis Hyatt To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: ping: [PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918] Message-ID: <20231130000547.GA62552@ldh-imac.local> References: <20231109211610.678626-1-lhyatt@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20231109211610.678626-1-lhyatt@gmail.com> X-Spam-Status: No, score=-3038.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org On Thu, Nov 09, 2023 at 04:16:10PM -0500, Lewis Hyatt wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918 > > This patch fixes the behavior of `#pragma GCC diagnostic pop' for permissive > error diagnostics such as -Wnarrowing (in C++11). Those currently do not > return to the correct state after the last pop; they become effectively > simple warnings instead. Bootstrap + regtest all languages on x86-64, does > it look OK please? Thanks! Hello- May I please ping this bug fix? https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635871.html Please note, it requires a trivial rebase on top of recent changes to the class diagnostic_context public interface. I attached the rebased patch here as well. Thanks! -Lewis When a diagnostic pragma changes the classification of a given diagnostic, the global options flags (such as warn_narrowing, etc.) may get changed too. Specifically, if a warning was not enabled initially and was later enabled by a pragma, then the corresponding global flag will change from false to true when the pragma is processed. That change is permanent and is not undone by a subsequent `#pragma GCC diagnostic pop'; the warning flag needs to remain enabled since a diagnostic could be generated later on for a source location prior to the pop. So in order to support popping to the initial classification, given that the global options flags no longer reflect that state, the diagnostic_context object itself remembers the way things were before it changed anything. The current implementation works fine for diagnostics that are always errors or always warnings, but it doesn't do the right thing for diagnostics that could be either, such as -Wnarrowing. The classification of that diagnostic (or any permerror diagnostic) depends on the state of -fpermissive; for the particular case of -Wnarrowing it also matters whether a compile-time or run-time narrowing is being diagnosed. The problem is that the current implementation insists on recording whether an enabled diagnostic should be a DK_WARNING or a DK_ERROR, and then, after popping to the initial state, it overrides it always to that type only. Fix that up by adding a new internal diagnostic type DK_ANY. This just indicates that the diagnostic is enabled without mandating exactly what type of diagnostic it should be. Then the diagnostic can be emitted with whatever type the frontend asks for. Incidentally, while making this change, I noticed that classify_diagnostic() spends some time computing a return value (the old classification kind) that is not used anywhere. The computed value seems to have some problems, mainly that it does not take into account `#pragma GCC diagnostic pop' at all, and so the returned value doesn't seem like it could make sense in many contexts. Given it would also not be desirable to leak the new internal-only DK_ANY type to outside callers, I think it would make sense in a subsequent cleanup patch to remove the return value altogether. gcc/ChangeLog: PR c++/111918 * diagnostic-core.h (enum diagnostic_t): Add DK_ANY special flag. * diagnostic.cc (diagnostic_option_classifier::classify_diagnostic): Make use of DK_ANY to indicate a diagnostic was initially enabled. (diagnostic_context::diagnostic_enabled): Do not change the type of a diagnostic if the saved classification is type DK_ANY. gcc/testsuite/ChangeLog: PR c++/111918 * g++.dg/cpp0x/Wnarrowing21a.C: New test. * g++.dg/cpp0x/Wnarrowing21b.C: New test. * g++.dg/cpp0x/Wnarrowing21c.C: New test. * g++.dg/cpp0x/Wnarrowing21d.C: New test. diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h index 04eba3d140e..4926c48da96 100644 --- a/gcc/diagnostic-core.h +++ b/gcc/diagnostic-core.h @@ -33,7 +33,10 @@ typedef enum DK_LAST_DIAGNOSTIC_KIND, /* This is used for tagging pragma pops in the diagnostic classification history chain. */ - DK_POP + DK_POP, + /* This is used internally to note that a diagnostic is enabled + without mandating any specific type. */ + DK_ANY, } diagnostic_t; /* RAII-style class for grouping related diagnostics. */ diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc index 4f66fa6acaa..fd40018a734 100644 --- a/gcc/diagnostic.cc +++ b/gcc/diagnostic.cc @@ -1136,8 +1136,7 @@ classify_diagnostic (const diagnostic_context *context, if (old_kind == DK_UNSPECIFIED) { old_kind = !context->option_enabled_p (option_index) - ? DK_IGNORED : (context->warning_as_error_requested_p () - ? DK_ERROR : DK_WARNING); + ? DK_IGNORED : DK_ANY; m_classify_diagnostic[option_index] = old_kind; } @@ -1472,7 +1471,15 @@ diagnostic_context::diagnostic_enabled (diagnostic_info *diagnostic) option. */ if (diag_class == DK_UNSPECIFIED && !option_unspecified_p (diagnostic->option_index)) - diagnostic->kind = m_option_classifier.get_current_override (diagnostic->option_index); + { + const diagnostic_t new_kind + = m_option_classifier.get_current_override (diagnostic->option_index); + if (new_kind != DK_ANY) + /* DK_ANY means the diagnostic is not to be ignored, but we don't want + to change it specifically to DK_ERROR or DK_WARNING; we want to + preserve whatever the caller has specified. */ + diagnostic->kind = new_kind; + } /* This allows for future extensions, like temporarily disabling warnings for ranges of source code. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21a.C b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21a.C new file mode 100644 index 00000000000..fa865987e5f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21a.C @@ -0,0 +1,14 @@ +/* PR c++/111918 */ +/* { dg-do compile { target c++11 } } */ +/* { dg-options "" } Suppress default -pedantic-errors so we can test permerror functionality. */ +extern int g (); +float f1{123456789}; /* { dg-error "-Wnarrowing" } */ +float f2{g ()}; /* { dg-warning "-Wnarrowing" } */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +float f3{123456789}; /* { dg-bogus "-Wnarrowing" } */ +float f4{g ()}; /* { dg-bogus "-Wnarrowing" } */ +#pragma GCC diagnostic pop +float f5{123456789}; /* { dg-bogus "warning" "warning in place of error" } */ + /* { dg-error "-Wnarrowing" "" { target *-*-* } .-1 } */ +float f6{g ()}; /* { dg-warning "-Wnarrowing" } */ diff --git a/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21b.C b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21b.C new file mode 100644 index 00000000000..b8049358acc --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21b.C @@ -0,0 +1,9 @@ +/* PR c++/111918 */ +/* { dg-do compile { target c++11 } } */ +/* { dg-options "-Wno-error=narrowing" } */ +float f1{123456789}; /* { dg-warning "-Wnarrowing" } */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +float f2{123456789}; /* { dg-bogus "-Wnarrowing" } */ +#pragma GCC diagnostic pop +float f3{123456789}; /* { dg-warning "-Wnarrowing" } */ diff --git a/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21c.C b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21c.C new file mode 100644 index 00000000000..6a98dd08176 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21c.C @@ -0,0 +1,9 @@ +/* PR c++/111918 */ +/* { dg-do compile { target c++11 } } */ +/* { dg-options "-fpermissive" } */ +float f1{123456789}; /* { dg-warning "-Wnarrowing" } */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +float f2{123456789}; /* { dg-bogus "-Wnarrowing" } */ +#pragma GCC diagnostic pop +float f3{123456789}; /* { dg-warning "-Wnarrowing" } */ diff --git a/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21d.C b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21d.C new file mode 100644 index 00000000000..a371e0a964f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing21d.C @@ -0,0 +1,9 @@ +/* PR c++/111918 */ +/* { dg-do compile { target c++11 } } */ +/* { dg-options "-Wno-narrowing" } */ +float f1{123456789}; /* { dg-bogus "-Wnarrowing" } */ +#pragma GCC diagnostic push +#pragma GCC diagnostic warning "-Wnarrowing" +float f2{123456789}; /* { dg-warning "-Wnarrowing" } */ +#pragma GCC diagnostic pop +float f3{123456789}; /* { dg-bogus "-Wnarrowing" } */