From patchwork Fri Oct 18 13:25:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lewis Hyatt X-Patchwork-Id: 1999145 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=L/+LYhOa; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XVQSf4l8mz1xvV for ; Sat, 19 Oct 2024 00:25:54 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5C8973858C39 for ; Fri, 18 Oct 2024 13:25:51 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qk1-x732.google.com (mail-qk1-x732.google.com [IPv6:2607:f8b0:4864:20::732]) by sourceware.org (Postfix) with ESMTPS id 9336C3858D37 for ; Fri, 18 Oct 2024 13:25:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9336C3858D37 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 9336C3858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::732 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729257934; cv=none; b=Koly/RHcI36rBTSpu8FBlQ8+WdOL8p+Bkl5SEvQiXCR5dvX3mSwAE9ipv5fAlx/Ym42Xx5ewPBw8xdNtd26OYdXOkXBZ46SoEWBNtkjHHb0e3lG0ecf/CZqM+2tSmnkQbxd3rM0Bcuwnt9O+2qrNtYkr2DxHFeMwQsxjICmR1P4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729257934; c=relaxed/simple; bh=50ReosAkMhqoMePp40G00neZTEKh0RqqqnUyMg13eos=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=XySbDKAzgEnlcuy7thlkdXWWtwbusjNXQa8XnNz20mkp1wiNGhSYLhM0tp5mjJ1bCsORQQiYJ7lbdV6yQ8AgzBII/laFCiHJUzSRbCS2GmGVHJhDWBss9waAU0igZIfRfMS9NcKqappPn9x0wdhPIcHcLUn+lzP7wq+VduySY30= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-qk1-x732.google.com with SMTP id af79cd13be357-7b1511697a5so121476785a.2 for ; Fri, 18 Oct 2024 06:25:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729257930; x=1729862730; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=8fVYq6PeS3Eu/Gicpzr5ZSLtCv/FIARVfp+Fg0PBz7w=; b=L/+LYhOaM/pGN9fBpW2aCDohHGJku31gXIQKx8snO53ye9xXfmgVRz0gAUegsPWMAL W/gmUgjEuopc35OwENAHP5xeinZxh6eYZ3VphrF3RW3VmikntV9jRSeJcL5KVA+CeYFo Lv9ykKFxPkGQbXcjjK/2MHbDLpb5mxzEN12/q43wsypxyYybcCYe1P/55G8gHiuKlLxh GpHaq7NWerUKEtrRcmgVlkTq7Yof8zbzlhbZ1k7wFUa6wQ9/c6L2TIc+wM7zOr84pzwO Qn992z93wwy1Bqr4tBKx2sUvY/UdY200BoYC7BEh3hWxxSytJJ64u5Kd4ZYt9AEgvJIW fKkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729257930; x=1729862730; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=8fVYq6PeS3Eu/Gicpzr5ZSLtCv/FIARVfp+Fg0PBz7w=; b=hinR8qgQ5Q0excYhZGBp3kisCmN0WwWPG12AcsMHyGQ23HtD+nNvEhGq+low9Cxm/Y tEpZTevE+LGFHTOtxO2P1b44y+lzBpS90m/z6CZYUOR2NuVMI+Ki2GbDm5IQODa36uYP HYa6Z8hKJNUHd52QV5UCa6WekEbL5FspB9P9G+xs/0lX50T+I9cvqxXWAGi32MDaMJRV k1gL29xRBiWJYZkFuG5m0Grjw0IVkGCLs19OD+CawZCmomFFESaBQ7esDU9flnxiYyw+ lgo/zdn8ThrfQMCXxAA7oiTGShCyx7ZUKbsfRdqu+g+HhJyofr6JOQzb7Mf2Qlhz8x02 H2WA== X-Gm-Message-State: AOJu0YwscVhaYxjSWf7LOY16eSUhIHq7IYiKGJFJDfhSBiGJnEJwwZ7G Wj9LQzR87LUVRkISxAm08bqVUc+NTK0Xaxm6laQekJYKtsUQiGBE7xlYmQ== X-Google-Smtp-Source: AGHT+IG9WSf/YrObFcJ5+u+0/ZrfGzMgcV03jvexhumc8mJ0HjXmRCCRd/MoxEVdDLN0HjQS7qP88A== X-Received: by 2002:a05:620a:4493:b0:7b1:4fdb:3a1c with SMTP id af79cd13be357-7b157be0cafmr315635085a.47.1729257929611; Fri, 18 Oct 2024 06:25:29 -0700 (PDT) Received: from localhost.localdomain ([173.54.240.58]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7b156fe57c8sm67953185a.96.2024.10.18.06.25.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Oct 2024 06:25:28 -0700 (PDT) From: Lewis Hyatt To: gcc-patches@gcc.gnu.org Cc: David Malcolm , Lewis Hyatt Subject: [PATCH] diagnostics: libcpp: Improve locations for _Pragma lexing diagnostics [PR114423] Date: Fri, 18 Oct 2024 09:25:20 -0400 Message-Id: <20241018132520.817073-1-lhyatt@gmail.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-3038.7 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 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~incoming=patchwork.ozlabs.org@gcc.gnu.org Hello- https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114423 The diagnostics we issue while lexing tokens from a _Pragma string have always come out at invalid locations. I had tried a couple years ago to fix this in a general way, but I think that ended up being too invasive a change to fix a problem that's pretty minor in practice, and it never got over the finish line. Here is a simple patch that improves the situation and addresses the recently filed PR on that topic, hopefully this incremental improvement is a better way to make some progress on this? It just adds a way for libcpp to override the location for all diagnostics temporarily, so that the diagnostics issued while lexing from a _Pragma string are issued at a real location (the location of the _Pragma token) and not a bogus one. That's a lot simpler than trying to arrange to produce valid locations when lexing tokens from an internal buffer. Bootstrap + regtest all languages on x86-64 Linux, tweaked a few existing tests to adjust to the new locations. OK for trunk? Thanks! -Lewis -- >8 -- libcpp is not currently set up to be able to generate valid locations for tokens lexed from a _Pragma string. Instead, after obtaining the tokens, it sets their locations all to the location of the _Pragma operator itself. This makes things like _Pragma("GCC diagnostic") work well enough, but if any diagnostics are issued during lexing, prior to resetting the token locations, those diagnostics get issued at the invalid locations. Fix that up by adding a new field pfile->diagnostic_override_loc that instructs libcpp to issue diagnostics at the alternate location. libcpp/ChangeLog: PR preprocessor/114423 * internal.h (struct cpp_reader): Add DIAGNOSTIC_OVERRIDE_LOC field. * directives.cc (destringize_and_run): Set the new field to the location of the _Pragma operator. * errors.cc (cpp_diagnostic_at): Support DIAGNOSTIC_OVERRIDE_LOC to temporarily issue diagnostics at a different location. (cpp_diagnostic_with_line): Likewise. gcc/testsuite/ChangeLog: PR preprocessor/114423 * c-c++-common/cpp/pragma-diagnostic-loc.c: New test. * c-c++-common/cpp/diagnostic-pragma-1.c: Adjust expected output. * g++.dg/pch/operator-1.C: Likewise. --- libcpp/directives.cc | 7 +++++++ libcpp/errors.cc | 19 +++++++++++++------ libcpp/internal.h | 4 ++++ .../c-c++-common/cpp/diagnostic-pragma-1.c | 9 ++++----- .../c-c++-common/cpp/pragma-diagnostic-loc.c | 17 +++++++++++++++++ gcc/testsuite/g++.dg/pch/operator-1.C | 8 +++++++- 6 files changed, 52 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc.c diff --git a/libcpp/directives.cc b/libcpp/directives.cc index 9d235fa1b05..5706c28b835 100644 --- a/libcpp/directives.cc +++ b/libcpp/directives.cc @@ -2430,6 +2430,12 @@ destringize_and_run (cpp_reader *pfile, const cpp_string *in, pfile->buffer->file = pfile->buffer->prev->file; pfile->buffer->sysp = pfile->buffer->prev->sysp; + /* See comment below regarding the use of expansion_loc as the location + for all tokens; arrange here that diagnostics issued during lexing + get the same treatment. */ + const auto prev_loc_override = pfile->diagnostic_override_loc; + pfile->diagnostic_override_loc = expansion_loc; + start_directive (pfile); _cpp_clean_line (pfile); save_directive = pfile->directive; @@ -2497,6 +2503,7 @@ destringize_and_run (cpp_reader *pfile, const cpp_string *in, make that applicable to the real buffer too. */ pfile->buffer->prev->sysp = pfile->buffer->sysp; _cpp_pop_buffer (pfile); + pfile->diagnostic_override_loc = prev_loc_override; /* Reset the old macro state before ... */ XDELETE (pfile->context); diff --git a/libcpp/errors.cc b/libcpp/errors.cc index ad45f61913c..96fc165c12a 100644 --- a/libcpp/errors.cc +++ b/libcpp/errors.cc @@ -60,13 +60,14 @@ cpp_diagnostic_at (cpp_reader * pfile, enum cpp_diagnostic_level level, enum cpp_warning_reason reason, rich_location *richloc, const char *msgid, va_list *ap) { - bool ret; - if (!pfile->cb.diagnostic) abort (); - ret = pfile->cb.diagnostic (pfile, level, reason, richloc, _(msgid), ap); - - return ret; + if (pfile->diagnostic_override_loc && level != CPP_DL_NOTE) + { + rich_location rc2{pfile->line_table, pfile->diagnostic_override_loc}; + return pfile->cb.diagnostic (pfile, level, reason, &rc2, _(msgid), ap); + } + return pfile->cb.diagnostic (pfile, level, reason, richloc, _(msgid), ap); } /* Print a diagnostic at the location of the previously lexed token. */ @@ -201,8 +202,14 @@ cpp_diagnostic_with_line (cpp_reader * pfile, enum cpp_diagnostic_level level, if (!pfile->cb.diagnostic) abort (); + /* Don't override note locations, which will likely make the note + more confusing. */ + const bool do_loc_override + = pfile->diagnostic_override_loc && level != CPP_DL_NOTE; + if (do_loc_override) + src_loc = pfile->diagnostic_override_loc; rich_location richloc (pfile->line_table, src_loc); - if (column) + if (column && !do_loc_override) richloc.override_column (column); ret = pfile->cb.diagnostic (pfile, level, reason, &richloc, _(msgid), ap); diff --git a/libcpp/internal.h b/libcpp/internal.h index a658a8c5739..13186c5a5b0 100644 --- a/libcpp/internal.h +++ b/libcpp/internal.h @@ -616,6 +616,10 @@ struct cpp_reader zero of said file. */ location_t main_loc; + /* If non-zero, override diagnostic locations (other than DK_NOTE + diagnostics) to this one. */ + location_t diagnostic_override_loc; + /* Returns true iff we should warn about UTF-8 bidirectional control characters. */ bool warn_bidi_p () const diff --git a/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-1.c b/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-1.c index 9867c94a8dd..6e37294fd2b 100644 --- a/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-1.c +++ b/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-1.c @@ -3,9 +3,8 @@ #pragma GCC warning "warn-a" // { dg-warning warn-a } #pragma GCC error "err-b" // { dg-error err-b } -#define CONST1 _Pragma("GCC warning \"warn-c\"") 1 -#define CONST2 _Pragma("GCC error \"err-d\"") 2 - -char a[CONST1]; // { dg-warning warn-c } -char b[CONST2]; // { dg-error err-d } +#define CONST1 _Pragma("GCC warning \"warn-c\"") 1 // { dg-warning warn-c } +#define CONST2 _Pragma("GCC error \"err-d\"") 2 // { dg-error err-d } +char a[CONST1]; // { dg-note "in expansion of macro 'CONST1'" } +char b[CONST2]; // { dg-note "in expansion of macro 'CONST2'" } diff --git a/gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc.c b/gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc.c new file mode 100644 index 00000000000..4ef40420cdd --- /dev/null +++ b/gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc.c @@ -0,0 +1,17 @@ +/* { dg-do preprocess } */ +/* PR preprocessor/114423 */ +/* Check that we now issue diagnostics at the location of the _Pragma + instead of an invalid location. If we someday manage to issue + diagnostics at better locations in the future, this will need + updating. */ +_Pragma("GCC warning \"warning1\"") /* { dg-warning "1:warning1" } */ +#define P _Pragma("GCC warning \"warning2\"") /* { dg-warning "11:warning2" } */ +P /* { dg-note "in expansion of macro" } */ +#define S "GCC warning \"warning3\"" +/**/ _Pragma(S) /* { dg-warning "6:warning3" } */ + +/* This diagnostic uses a different code path (cpp_diagnostic_at() rather + than cpp_error_with_line()). Also make sure that the dg-note location + does not get overridden to the _Pragma location. */ +#pragma GCC poison xyz /* { dg-note "poisoned here" } */ +/* */ _Pragma("xyz") /* { dg-error "7:attempt to use poisoned" } */ diff --git a/gcc/testsuite/g++.dg/pch/operator-1.C b/gcc/testsuite/g++.dg/pch/operator-1.C index 290b5f7ab21..1138a969536 100644 --- a/gcc/testsuite/g++.dg/pch/operator-1.C +++ b/gcc/testsuite/g++.dg/pch/operator-1.C @@ -1,2 +1,8 @@ #include "operator-1.H" -int main(void){ major(0);} /* { dg-warning "Did not Work" } */ +int main(void){ major(0);} /* { dg-note "in expansion of macro 'major'" } */ +/* Line numbers below pertain to the header file. */ +/* { dg-warning "Did not Work" "" { target *-*-* } 1 } */ +/* { dg-note "in expansion of macro '__glibc_macro_warning1'" "" { target *-*-* } 3 } */ +/* { dg-note "in expansion of macro '__glibc_macro_warning'" "" { target *-*-* } 4 } */ +/* { dg-note "in expansion of macro '__SYSMACROS_DM1'" "" { target *-*-* } 6 } */ +/* { dg-note "in expansion of macro '__SYSMACROS_DM'" "" { target *-*-* } 9 } */