From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) by sourceware.org (Postfix) with ESMTPS id E7F0738582A7 for ; Sat, 9 Jul 2022 20:52:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E7F0738582A7 Received: by mail-qk1-x735.google.com with SMTP id p22so1388843qkj.4 for ; Sat, 09 Jul 2022 13:52:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=Jt5xQyHqKIZ3/SvgN0OzY2l7jFCjQF2ty9IBKIidqxU=; b=huLWZsXIXDQRQ6+Is3e7xKaLicrEtK3Z4mhge0kCVrmJ9DiGO8IZXLFcFP/7BjC51f 7dOh318MPosGxiqizzF0cTN/w+BXynRGHI+YfjDOh8MZGXgM+h3wHgr1SZPxPOy4Fi/I GdRdNSV4OYMod2yEQJBxsVCulFPr9B8SRiEINo1jzfctGpjgw2/5caliaEude2tPvU0v TJ0Q4Yng5/XTZO+GNyTsW6AivQ1u03mIA1VRfcNpCM9vmJlbnChuvm/VEc022fraO8dN Kus1Y+eVqlGsnOMKx2owRhkhVKZfbpk0/tlTDcFcthS2VQiooCO7mAke8vOr9631UgL0 nUDA== X-Gm-Message-State: AJIora8zODSu7XfmxG6D3sTMSFB/79gVxgF/z5XUbZSZuK0zrIJ8WNZV wlCALIMyKomnV5dPvEgRxwncbbwBy7E= X-Google-Smtp-Source: AGRyM1vZlFV0WOEBbuy/fgjkDpB4KPTNQQi1T2FkpRvesx1ZtHg5cinZd8Yf8ktVSFju7lh0oZdCng== X-Received: by 2002:a05:620a:2894:b0:6ae:ba71:ea98 with SMTP id j20-20020a05620a289400b006aeba71ea98mr6653199qkp.89.1657399952849; Sat, 09 Jul 2022 13:52:32 -0700 (PDT) Received: from ldh-imac.local (pool-173-70-35-242.nwrknj.fios.verizon.net. [173.70.35.242]) by smtp.gmail.com with ESMTPSA id r19-20020a05622a035300b00304f7449e17sm2114175qtw.93.2022.07.09.13.52.31 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sat, 09 Jul 2022 13:52:31 -0700 (PDT) Date: Sat, 9 Jul 2022 16:52:30 -0400 From: Lewis Hyatt To: gcc-patches@gcc.gnu.org Subject: [PATCH] c: Fix location for _Pragma tokens [PR97498] Message-ID: <20220709205230.GA4991@ldh-imac.local> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="opJtzjQTFsWo+cga" Content-Disposition: inline User-Agent: Mutt/1.12.1 (2019-06-15) X-Spam-Status: No, score=-3039.2 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.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 Jul 2022 20:52:36 -0000 --opJtzjQTFsWo+cga Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hello- PR97498 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97498) is another PR related to the fact that imprecise locations for _Pragma result in counterintuitive behavior for GCC diagnostic pragmas, which inhibit the ability to make convenient wrapper macros for enabling and disabling diagnostics in specific scopes. It looks like David did a lot of work a few years ago improving this (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69543 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69558), and in particular r233637 added a lot of new test coverage for cases that had regressed in the past. I think the main source of problems for all remaining issues is that we use the global input_location for deciding when/if a diagnostic should apply. I think it should be eventually doable to eliminate this, and rather properly resolve the token locations to the place they need to be so that _Pragma type wrapper macros just work the way people expect. That said, PR97498 can be solved easily with a 2-line fix without removing input_location, and I think the resulting change to input_location's value is an improvement that will benefit other areas, so I thought I'd see what you think about this patch please? Here is a typical testcase. Note the line continuations so it's all one logical line. === _Pragma("GCC diagnostic push") \ _Pragma("GCC diagnostic ignored \"-Wunused-function\"") \ static void f() {} \ _Pragma("GCC diagnostic pop") === What happens is that in C++ mode, input_location is always updated to the most recently-lexed token, so the above case works fine and does not warn when compiled with "g++ -Wunused-functions". However, in C mode, it does warn because input_location in C is almost always set to the start of the line, and is in this case. So the pop is deemed to take place prior to the definition of f(). Initially, I thought it best to change input_location for C mode to behave like C++, and always update to the most recently lexed token. Maybe that's still the right way to go, but there was a fair amount of testsuite fallout from that. Most of it, was just that we would need to change the tests to look for the new locations, and in many cases, the new locations seemed preferable to the old ones, but it seemed a bit much for now, so I took a more measured approach and just changed input_location in the specific case of processing a pragma, to be the location of the CPP_PRAGMA token. Unfortunately, it turns out that the CPP_PRAGMA token that libcpp provides to represent the _Pragma() expression doesn't have a valid location with which input_location could be overridden. Looking into that, in r232893 David added logic which sets the location of all tokens inside the _Pragma(...) to a reasonable place (namely it points to "_Pragma" at the expansion point). However, that patch didn't change the location of the CPP_PRAGMA token itself to similarly point there, so the 2nd line of this patch does that. The rest of it is just tweaking a couple tests which were sensitive to the location being output. In all these cases, the new locations seem more informative to me than the old ones. With those tweaks, bootstrap + regtest all languages looks good with no regressions. Please let me know what you think? Thanks! -Lewis --opJtzjQTFsWo+cga Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="pr97498.txt" [PATCH] c: Fix location for _Pragma tokens [PR97498] The handling of #pragma GCC diagnostic uses input_location, which is not always as precise as needed; in particular the relative location of some tokens and a _Pragma directive will crucially determine whether a given diagnostic is enabled or suppressed in the desired way. PR97498 shows how the C frontend ends up with input_location pointing to the beginning of the line containing a _Pragma() directive, resulting in the wrong behavior if the diagnostic to be modified pertains to some tokens found earlier on the same line. This patch fixes that by addressing two issues: a) libcpp was not assigning a valid location to the CPP_PRAGMA token generated by the _Pragma directive. b) C frontend was not setting input_location to something reasonable. With this change, the C frontend is able to change input_location to point to the _Pragma token as needed. This is just a two-line fix (one for each of a) and b)), the testsuite changes were needed only because the location on the tested warnings has been somewhat improved, so the tests need to look for the new locations. gcc/c/ChangeLog: PR preprocessor/97498 * c-parser.cc (c_parser_pragma): Set input_location to the location of the pragma, rather than the start of the line. libcpp/ChangeLog: PR preprocessor/97498 * directives.cc (destringize_and_run): Override the location of the CPP_PRAGMA token from a _Pragma directive to the location of the expansion point, as is done for the tokens lexed from it. gcc/testsuite/ChangeLog: PR preprocessor/97498 * c-c++-common/pr97498.c: New test. * c-c++-common/gomp/pragma-3.c: Adapt for improved warning locations. * c-c++-common/gomp/pragma-5.c: Likewise. * gcc.dg/pragma-message.c: Likewise. libgomp/ChangeLog: * testsuite/libgomp.oacc-c-c++-common/reduction-5.c: Adapt for improved warning locations. * testsuite/libgomp.oacc-c-c++-common/vred2d-128.c: Likewise. diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index 9c02141e2c6..92049d1a101 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -12397,6 +12397,7 @@ c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p) unsigned int id; const char *construct = NULL; + input_location = c_parser_peek_token (parser)->location; id = c_parser_peek_token (parser)->pragma_kind; gcc_assert (id != PRAGMA_NONE); diff --git a/gcc/testsuite/c-c++-common/gomp/pragma-3.c b/gcc/testsuite/c-c++-common/gomp/pragma-3.c index c1dee1bcc62..ae18e9b8886 100644 --- a/gcc/testsuite/c-c++-common/gomp/pragma-3.c +++ b/gcc/testsuite/c-c++-common/gomp/pragma-3.c @@ -1,13 +1,14 @@ /* { dg-additional-options "-fdump-tree-original" } */ /* PR preprocessor/103165 */ -#define inner(...) #__VA_ARGS__ ; _Pragma("omp error severity(warning) message (\"Test\") at(compilation)") +#define inner(...) #__VA_ARGS__ ; _Pragma("omp error severity(warning) message (\"Test\") at(compilation)") /* { dg-line inner_location } */ #define outer(...) inner(__VA_ARGS__) void f (void) { - const char *str = outer(inner(1,2)); /* { dg-warning "'pragma omp error' encountered: Test" } */ + const char *str = outer(inner(1,2)); + /* { dg-warning "'pragma omp error' encountered: Test" "inner expansion" { target *-*-* } inner_location } */ } #if 0 diff --git a/gcc/testsuite/c-c++-common/gomp/pragma-5.c b/gcc/testsuite/c-c++-common/gomp/pragma-5.c index af54b682789..8124f701502 100644 --- a/gcc/testsuite/c-c++-common/gomp/pragma-5.c +++ b/gcc/testsuite/c-c++-common/gomp/pragma-5.c @@ -1,13 +1,14 @@ /* { dg-additional-options "-fdump-tree-original" } */ /* PR preprocessor/103165 */ -#define inner(...) #__VA_ARGS__ ; _Pragma ( " omp error severity (warning) message (\"Test\") at(compilation)" ) +#define inner(...) #__VA_ARGS__ ; _Pragma ( " omp error severity (warning) message (\"Test\") at(compilation)" ) /* { dg-line inner_location } */ #define outer(...) inner(__VA_ARGS__) void f (void) { - const char *str = outer(inner(1,2)); /* { dg-warning "'pragma omp error' encountered: Test" } */ + const char *str = outer(inner(1,2)); + /* { dg-warning "'pragma omp error' encountered: Test" "inner expansion" { target *-*-* } inner_location } */ } #if 0 diff --git a/gcc/testsuite/c-c++-common/pr97498.c b/gcc/testsuite/c-c++-common/pr97498.c new file mode 100644 index 00000000000..f5fa420415b --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr97498.c @@ -0,0 +1,4 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-Wunused-function" } */ +#pragma GCC diagnostic ignored "-Wunused-function" +static void f() {} _Pragma("GCC diagnostic error \"-Wunused-function\"") /* { dg-bogus "-Wunused-function" } */ diff --git a/gcc/testsuite/gcc.dg/pragma-message.c b/gcc/testsuite/gcc.dg/pragma-message.c index 2f44b617710..1b7cf09de0a 100644 --- a/gcc/testsuite/gcc.dg/pragma-message.c +++ b/gcc/testsuite/gcc.dg/pragma-message.c @@ -42,9 +42,11 @@ #pragma message ("Okay " THREE) /* { dg-message "Okay 3" } */ /* Create a TODO() that prints a message on compilation. */ -#define DO_PRAGMA(x) _Pragma (#x) -#define TODO(x) DO_PRAGMA(message ("TODO - " #x)) -TODO(Okay 4) /* { dg-message "TODO - Okay 4" } */ +#define DO_PRAGMA(x) _Pragma (#x) /* { dg-line pragma_loc1 } */ +#define TODO(x) DO_PRAGMA(message ("TODO - " #x)) /* { dg-line pragma_loc2 } */ +TODO(Okay 4) /* { dg-message "in expansion of macro 'TODO'" } */ +/* { dg-message "TODO - Okay 4" "test4.1" { target *-*-* } pragma_loc1 } */ +/* { dg-message "in expansion of macro 'DO_PRAGMA'" "test4.2" { target *-*-* } pragma_loc2 } */ #if 0 #pragma message ("Not printed") diff --git a/libcpp/directives.cc b/libcpp/directives.cc index f804a441f39..4104d5166e2 100644 --- a/libcpp/directives.cc +++ b/libcpp/directives.cc @@ -1930,6 +1930,7 @@ destringize_and_run (cpp_reader *pfile, const cpp_string *in, maxcount = 50; toks = XNEWVEC (cpp_token, maxcount); toks[0] = pfile->directive_result; + toks[0].src_loc = expansion_loc; do { diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c index bae1dee6ad2..16aa0dd4ac1 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c @@ -12,7 +12,7 @@ const int n = 100; -#define DO_PRAGMA(x) _Pragma (#x) +#define DO_PRAGMA(x) _Pragma (#x) /* { dg-line pragma_loc } */ #define check_reduction(gwv_par, gwv_loop) \ { \ @@ -46,7 +46,7 @@ main (void) /* Nvptx targets require a vector_length or 32 in to allow spinlocks with gangs. */ check_reduction (num_workers (nw) vector_length (vl), worker); - /* { dg-warning "region is vector partitioned but does not contain vector partitioned code" "" { target *-*-* } .-1 } */ + /* { dg-warning "region is vector partitioned but does not contain vector partitioned code" "test1" { target *-*-* } pragma_loc } */ check_reduction (vector_length (vl), vector); check_reduction (num_gangs (ng) num_workers (nw) vector_length (vl), gang worker vector); diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/vred2d-128.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/vred2d-128.c index 9c182d90a0d..84e6d51670b 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/vred2d-128.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/vred2d-128.c @@ -17,7 +17,7 @@ int a1[n], a2[n]; void name () \ { \ long i, j, t1, t2, t3; /* { dg-line vars } */ \ - _Pragma(outer) \ + _Pragma(outer) /* { dg-line outer } */ \ for (i = 0; i < n; i++) \ { \ t1 = 0; \ @@ -40,44 +40,44 @@ int a1[n], a2[n]; gentest (test1, "acc parallel loop gang vector_length (128) firstprivate (t1, t2)", "acc loop vector reduction(+:t1) reduction(-:t2)") -/* { dg-warning {'t1' is used uninitialized} {} { target *-*-* } .-1 } +/* { dg-warning {'t1' is used uninitialized} {} { target *-*-* } outer } { dg-note {'t1' was declared here} {} { target *-*-* } vars } { dg-note {in expansion of macro 'gentest'} {} { target { ! offloading_enabled } } .-4 } TODO See PR101551 for 'offloading_enabled' differences. */ -/* { dg-warning {'t2' is used uninitialized} {} { target *-*-* } .-5 } +/* { dg-warning {'t2' is used uninitialized} {} { target *-*-* } outer } { dg-note {'t2' was declared here} {} { target *-*-* } vars } { DUP_dg-note {in expansion of macro 'gentest'} {} { target { ! offloading_enabled } } .-8 } TODO See PR101551 for 'offloading_enabled' differences. */ gentest (test2, "acc parallel loop gang vector_length (128) firstprivate (t1, t2)", "acc loop worker vector reduction(+:t1) reduction(-:t2)") -/* { dg-warning {'t1' is used uninitialized} {} { target *-*-* } .-1 } +/* { DUPdg-warning {'t1' is used uninitialized} {} { target *-*-* } outer } { DUP_dg-note {'t1' was declared here} {} { target *-*-* } vars } { dg-note {in expansion of macro 'gentest'} {} { target { ! offloading_enabled } } .-4 } TODO See PR101551 for 'offloading_enabled' differences. */ -/* { dg-warning {'t2' is used uninitialized} {} { target *-*-* } .-5 } +/* { DUPdg-warning {'t2' is used uninitialized} {} { target *-*-* } outer } { DUP_dg-note {'t2' was declared here} {} { target *-*-* } vars } { DUP_dg-note {in expansion of macro 'gentest'} {} { target { ! offloading_enabled } } .-8 } TODO See PR101551 for 'offloading_enabled' differences. */ gentest (test3, "acc parallel loop gang worker vector_length (128) firstprivate (t1, t2)", "acc loop vector reduction(+:t1) reduction(-:t2)") -/* { dg-warning {'t1' is used uninitialized} {} { target *-*-* } .-1 } +/* { DUPdg-warning {'t1' is used uninitialized} {} { target *-*-* } outer } { DUP_dg-note {'t1' was declared here} {} { target *-*-* } vars } { dg-note {in expansion of macro 'gentest'} {} { target { ! offloading_enabled } } .-4 } TODO See PR101551 for 'offloading_enabled' differences. */ -/* { dg-warning {'t2' is used uninitialized} {} { target *-*-* } .-5 } +/* { DUPdg-warning {'t2' is used uninitialized} {} { target *-*-* } outer } { DUP_dg-note {'t2' was declared here} {} { target *-*-* } vars } { DUP_dg-note {in expansion of macro 'gentest'} {} { target { ! offloading_enabled } } .-8 } TODO See PR101551 for 'offloading_enabled' differences. */ gentest (test4, "acc parallel loop firstprivate (t1, t2)", "acc loop reduction(+:t1) reduction(-:t2)") -/* { dg-warning {'t1' is used uninitialized} {} { target *-*-* } .-1 } +/* { DUPdg-warning {'t1' is used uninitialized} {} { target *-*-* } outer } { DUP_dg-note {'t1' was declared here} {} { target *-*-* } vars } { dg-note {in expansion of macro 'gentest'} {} { target { ! offloading_enabled } } .-4 } TODO See PR101551 for 'offloading_enabled' differences. */ -/* { dg-warning {'t2' is used uninitialized} {} { target *-*-* } .-5 } +/* { DUPdg-warning {'t2' is used uninitialized} {} { target *-*-* } outer } { DUP_dg-note {'t2' was declared here} {} { target *-*-* } vars } { DUP_dg-note {in expansion of macro 'gentest'} {} { target { ! offloading_enabled } } .-8 } TODO See PR101551 for 'offloading_enabled' differences. */ --opJtzjQTFsWo+cga--