c++: Fix location for -Wunused-macros [PR66290]
Commit Message
In C++, since all tokens are lexed from libcpp up front, diagnostics generated
by libcpp after lexing has completed do not get a valid location from libcpp
(rather, libcpp thinks they all pertain to the end of the file.) This has long
been addressed using the global variable "done_lexing", which the C++ frontend
sets at the appropriate time; when done_lexing is true, then c_cpp_diagnostic(),
which outputs libcpp's diagnostics, uses input_location instead of the wrong
libcpp location. The C++ frontend arranges that input_location will point to the
token it is currently processing, so this generally works fine. However, there
is one exception currently, which is -Wunused-macros. This gets generated at the
end of processing in cpp_finish (), since we need to wait until then to
determine whether a macro was eventually used or not. But the locations it
passes to c_cpp_diagnostic () were remembered from the original lexing and hence
they should not be overridden with input_location, which is now the one
incorrectly pointing to the end of the file.
Fixed by setting done_lexing=false again just prior to calling cpp_finish (). I
also renamed the variable from done_lexing to "override_libcpp_locations", since
it's now not strictly about lexing anymore.
There is no new testcase with this patch, since we already had an xfailed
testcase which is now fixed.
gcc/c-family/ChangeLog:
PR c++/66290
* c-common.h: Rename global done_lexing to
override_libcpp_locations.
* c-common.cc (c_cpp_diagnostic): Likewise.
* c-opts.cc (c_common_finish): Set override_libcpp_locations
(formerly done_lexing) immediately prior to calling cpp_finish ().
gcc/cp/ChangeLog:
PR c++/66290
* parser.cc (cp_lexer_new_main): Rename global done_lexing to
override_libcpp_locations.
gcc/testsuite/ChangeLog:
PR c++/66290
* c-c++-common/pragma-diag-15.c: Remove xfail for C++.
---
Notes:
Hello-
The attached patch fixes PR66290, which is about C++ diagnostics using
the wrong location for -Wunused-macros. Please let me know if it looks OK?
Bootstrap + regtest all languages on x86-64 linux looks good, no changes other
than the un-XFAILed testcases.
Thank you!
-Lewis
gcc/c-family/c-common.cc | 10 ++++++----
gcc/c-family/c-common.h | 8 +++++---
gcc/c-family/c-opts.cc | 6 ++++++
gcc/cp/parser.cc | 2 +-
gcc/testsuite/c-c++-common/pragma-diag-15.c | 2 +-
5 files changed, 19 insertions(+), 9 deletions(-)
Comments
On 7/28/2022 8:01 AM, Lewis Hyatt via Gcc-patches wrote:
> In C++, since all tokens are lexed from libcpp up front, diagnostics generated
> by libcpp after lexing has completed do not get a valid location from libcpp
> (rather, libcpp thinks they all pertain to the end of the file.) This has long
> been addressed using the global variable "done_lexing", which the C++ frontend
> sets at the appropriate time; when done_lexing is true, then c_cpp_diagnostic(),
> which outputs libcpp's diagnostics, uses input_location instead of the wrong
> libcpp location. The C++ frontend arranges that input_location will point to the
> token it is currently processing, so this generally works fine. However, there
> is one exception currently, which is -Wunused-macros. This gets generated at the
> end of processing in cpp_finish (), since we need to wait until then to
> determine whether a macro was eventually used or not. But the locations it
> passes to c_cpp_diagnostic () were remembered from the original lexing and hence
> they should not be overridden with input_location, which is now the one
> incorrectly pointing to the end of the file.
>
> Fixed by setting done_lexing=false again just prior to calling cpp_finish (). I
> also renamed the variable from done_lexing to "override_libcpp_locations", since
> it's now not strictly about lexing anymore.
>
> There is no new testcase with this patch, since we already had an xfailed
> testcase which is now fixed.
>
> gcc/c-family/ChangeLog:
>
> PR c++/66290
> * c-common.h: Rename global done_lexing to
> override_libcpp_locations.
> * c-common.cc (c_cpp_diagnostic): Likewise.
> * c-opts.cc (c_common_finish): Set override_libcpp_locations
> (formerly done_lexing) immediately prior to calling cpp_finish ().
>
> gcc/cp/ChangeLog:
>
> PR c++/66290
> * parser.cc (cp_lexer_new_main): Rename global done_lexing to
> override_libcpp_locations.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/66290
> * c-c++-common/pragma-diag-15.c: Remove xfail for C++.
Ewww. But OK.
Jeff
@@ -284,9 +284,11 @@ int c_inhibit_evaluation_warnings;
be generated. */
bool in_late_binary_op;
-/* Whether lexing has been completed, so subsequent preprocessor
- errors should use the compiler's input_location. */
-bool done_lexing = false;
+/* Depending on which phase of processing we are in, we may need
+ to prefer input_location to libcpp's locations. (Specifically,
+ after the C++ lexer is done lexing tokens, but prior to calling
+ cpp_finish (), we need to do so. */
+bool override_libcpp_locations;
/* Information about how a function name is generated. */
struct fname_var_t
@@ -6681,7 +6683,7 @@ c_cpp_diagnostic (cpp_reader *pfile ATTRIBUTE_UNUSED,
default:
gcc_unreachable ();
}
- if (done_lexing)
+ if (override_libcpp_locations)
richloc->set_range (0, input_location, SHOW_RANGE_WITH_CARET);
diagnostic_set_info_translated (&diagnostic, msg, ap,
richloc, dlevel);
@@ -767,10 +767,12 @@ extern int max_tinst_depth;
extern int c_inhibit_evaluation_warnings;
-/* Whether lexing has been completed, so subsequent preprocessor
- errors should use the compiler's input_location. */
+/* Depending on which phase of processing we are in, we may need
+ to prefer input_location to libcpp's locations. (Specifically,
+ after the C++ lexer is done lexing tokens, but prior to calling
+ cpp_finish (), we need to do so. */
-extern bool done_lexing;
+extern bool override_libcpp_locations;
/* C types are partitioned into three subsets: object, function, and
incomplete types. */
@@ -1281,6 +1281,12 @@ c_common_finish (void)
}
}
+ /* When we call cpp_finish (), it may generate some diagnostics using
+ locations it remembered from the preprocessing phase, e.g. for
+ -Wunused-macros. So inform c_cpp_diagnostic () not to override those
+ locations with input_location, which would be incorrect now. */
+ override_libcpp_locations = false;
+
/* For performance, avoid tearing down cpplib's internal structures
with cpp_destroy (). */
cpp_finish (parse_in, deps_stream);
@@ -755,7 +755,7 @@ cp_lexer_new_main (void)
/* Subsequent preprocessor diagnostics should use compiler
diagnostic functions to get the compiler source location. */
- done_lexing = true;
+ override_libcpp_locations = true;
maybe_check_all_macros (parse_in);
@@ -9,5 +9,5 @@
because the location of the macro definition is incorrectly set. This is a
separate issue, will resolve it in a later patch. */
-#define X /* { dg-warning "-:-Wunused-macros" {} { xfail c++ } } */
+#define X /* { dg-warning "-:-Wunused-macros" } */
#pragma GCC diagnostic ignored "-Wunused-macros"