[PATCH] diagnostics: fix end-points of ranges within macros (PR c++/79300)
David Malcolm
dmalcolm@redhat.com
Thu Feb 2 20:18:00 GMT 2017
PR c++/79300 identifies an issue in which diagnostics_show_locus
prints the wrong end-point for a range within a macro:
assert ((p + val_size) - buf == encoded_len);
~~~~~~~~~~~~~~~~~~~~~^~~~
as opposed to:
assert ((p + val_size) - buf == encoded_len);
~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
The caret, start and finish locations of this compound location are
all virtual locations.
The root cause is that when diagnostic-show-locus.c's layout ctor
expands the caret and end-points, it calls
linemap_client_expand_location_to_spelling_point
which (via expand_location_1) unwinds the macro expansions, and
then calls linemap_expand_location. Doing so implicitly picks the
*caret* location for any virtual locations, and so in the above case
it picks these spelling locations for the three parts of the location:
assert ((p + val_size) - buf == encoded_len);
^ ^ ^
START | FINISH
CARET
and so erroneously strips the underlining from the final token, apart
from its first character.
The fix is for layout's ctor to indicate that it wants the start/finish
locations in such a situation, adding a new param to
linemap_client_expand_location_to_spelling_point, so that
expand_location_1 can handle this case by extracting the relevant part
of the unwound compound location, and thus choose:
assert ((p + val_size) - buf == encoded_len);
^ ^ ^
START | FINISH
CARET
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for stage 4, or should I wait until stage 1?
gcc/ChangeLog:
PR c++/79300
* diagnostic-show-locus.c (layout::layout): Use start and finish
spelling location for the start and finish of each range.
* genmatch.c (linemap_client_expand_location_to_spelling_point):
Add unused aspect param.
* input.c (expand_location_1): Add "aspect" param, and use it
to access the correct part of the location.
(expand_location): Pass LOCATION_ASPECT_CARET to new param of
expand_location_1.
(expand_location_to_spelling_point): Likewise.
(linemap_client_expand_location_to_spelling_point): Add "aspect"
param, and pass it to expand_location_1.
gcc/testsuite/ChangeLog:
PR c++/79300
* c-c++-common/Wmisleading-indentation-3.c (fn_14): Update
expected underlining within macro expansion.
* c-c++-common/pr70264.c: Likewise.
* g++.dg/plugin/diagnostic-test-expressions-1.C
(test_within_macro_1): New test.
(test_within_macro_2): Likewise.
(test_within_macro_3): Likewise.
(test_within_macro_4): Likewise.
* gcc.dg/format/diagnostic-ranges.c (test_macro_3): Update
expected underlining within macro expansion.
(test_macro_4): Likewise.
* gcc.dg/plugin/diagnostic-test-expressions-1.c
(test_within_macro_1): New test.
(test_within_macro_2): Likewise.
(test_within_macro_3): Likewise.
(test_within_macro_4): Likewise.
* gcc.dg/spellcheck-fields-2.c (test_macro): Update expected
underlining within macro expansion.
libcpp/ChangeLog:
PR c++/79300
* include/line-map.h (enum location_aspect): New enum.
(linemap_client_expand_location_to_spelling_point): Add
enum location_aspect param.
* line-map.c (source_range::intersects_line_p): Update for new
param of linemap_client_expand_location_to_spelling_point.
(rich_location::get_expanded_location): Likewise.
(fixit_insert::affects_line_p): Likewise.
---
gcc/diagnostic-show-locus.c | 9 ++-
gcc/genmatch.c | 3 +-
gcc/input.c | 52 +++++++++++---
.../c-c++-common/Wmisleading-indentation-3.c | 2 +-
gcc/testsuite/c-c++-common/pr70264.c | 2 +-
.../g++.dg/plugin/diagnostic-test-expressions-1.C | 78 +++++++++++++++++++++
gcc/testsuite/gcc.dg/format/diagnostic-ranges.c | 4 +-
.../gcc.dg/plugin/diagnostic-test-expressions-1.c | 79 ++++++++++++++++++++++
gcc/testsuite/gcc.dg/spellcheck-fields-2.c | 2 +-
libcpp/include/line-map.h | 12 +++-
libcpp/line-map.c | 15 ++--
11 files changed, 234 insertions(+), 24 deletions(-)
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 5c386ae..a0914ef 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -784,11 +784,14 @@ layout::layout (diagnostic_context * context,
/* Expand the various locations. */
expanded_location start
- = linemap_client_expand_location_to_spelling_point (src_range.m_start);
+ = linemap_client_expand_location_to_spelling_point
+ (src_range.m_start, LOCATION_ASPECT_START);
expanded_location finish
- = linemap_client_expand_location_to_spelling_point (src_range.m_finish);
+ = linemap_client_expand_location_to_spelling_point
+ (src_range.m_finish, LOCATION_ASPECT_FINISH);
expanded_location caret
- = linemap_client_expand_location_to_spelling_point (loc_range->m_loc);
+ = linemap_client_expand_location_to_spelling_point
+ (loc_range->m_loc, LOCATION_ASPECT_CARET);
/* If any part of the range isn't in the same file as the primary
location of this diagnostic, ignore the range. */
diff --git a/gcc/genmatch.c b/gcc/genmatch.c
index c163ded..3723c7c 100644
--- a/gcc/genmatch.c
+++ b/gcc/genmatch.c
@@ -61,7 +61,8 @@ static struct line_maps *line_table;
This is the implementation for genmatch. */
expanded_location
-linemap_client_expand_location_to_spelling_point (source_location loc)
+linemap_client_expand_location_to_spelling_point (source_location loc,
+ enum location_aspect)
{
const struct line_map_ordinary *map;
loc = linemap_resolve_location (line_table, loc, LRK_SPELLING_LOCATION, &map);
diff --git a/gcc/input.c b/gcc/input.c
index 3e67314..07ca5da 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -147,11 +147,14 @@ static const size_t fcache_line_record_size = 100;
associated line/column) in the context of a macro expansion, the
returned location is the first one (while unwinding the macro
location towards its expansion point) that is in real source
- code. */
+ code.
+
+ ASPECT controls which part of the location to use. */
static expanded_location
expand_location_1 (source_location loc,
- bool expansion_point_p)
+ bool expansion_point_p,
+ enum location_aspect aspect)
{
expanded_location xloc;
const line_map_ordinary *map;
@@ -181,8 +184,36 @@ expand_location_1 (source_location loc,
loc, NULL);
lrk = LRK_SPELLING_LOCATION;
}
- loc = linemap_resolve_location (line_table, loc,
- lrk, &map);
+ loc = linemap_resolve_location (line_table, loc, lrk, &map);
+
+ /* loc is now either in an ordinary map, or is a reserved location.
+ If it is a compound location, the caret is in a spelling location,
+ but the start/finish might still be a virtual location.
+ Depending of what the caller asked for, we may need to recurse
+ one level in order to resolve any virtual locations in the
+ end-points. */
+ switch (aspect)
+ {
+ default:
+ gcc_unreachable ();
+ /* Fall through. */
+ case LOCATION_ASPECT_CARET:
+ break;
+ case LOCATION_ASPECT_START:
+ {
+ source_location start = get_start (loc);
+ if (start != loc)
+ return expand_location_1 (start, expansion_point_p, aspect);
+ }
+ break;
+ case LOCATION_ASPECT_FINISH:
+ {
+ source_location finish = get_finish (loc);
+ if (finish != loc)
+ return expand_location_1 (finish, expansion_point_p, aspect);
+ }
+ break;
+ }
xloc = linemap_expand_location (line_table, map, loc);
}
@@ -773,7 +804,8 @@ is_location_from_builtin_token (source_location loc)
expanded_location
expand_location (source_location loc)
{
- return expand_location_1 (loc, /*expansion_point_p=*/true);
+ return expand_location_1 (loc, /*expansion_point_p=*/true,
+ LOCATION_ASPECT_CARET);
}
/* Expand the source location LOC into a human readable location. If
@@ -785,7 +817,8 @@ expand_location (source_location loc)
expanded_location
expand_location_to_spelling_point (source_location loc)
{
- return expand_location_1 (loc, /*expansion_point_p=*/false);
+ return expand_location_1 (loc, /*expansion_point_p=*/false,
+ LOCATION_ASPECT_CARET);
}
/* The rich_location class within libcpp requires a way to expand
@@ -795,12 +828,13 @@ expand_location_to_spelling_point (source_location loc)
to do this.
This is the implementation for libcommon.a (all host binaries),
- which simply calls into expand_location_to_spelling_point. */
+ which simply calls into expand_location_1. */
expanded_location
-linemap_client_expand_location_to_spelling_point (source_location loc)
+linemap_client_expand_location_to_spelling_point (source_location loc,
+ enum location_aspect aspect)
{
- return expand_location_to_spelling_point (loc);
+ return expand_location_1 (loc, /*expansion_point_p=*/false, aspect);
}
diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation-3.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation-3.c
index 6482b00..870ba72 100644
--- a/gcc/testsuite/c-c++-common/Wmisleading-indentation-3.c
+++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation-3.c
@@ -68,7 +68,7 @@ void fn_14 (void)
/* { dg-begin-multiline-output "" }
for ((VAR) = (START); (VAR) < (STOP); (VAR++))
- ^
+ ^~~
{ dg-end-multiline-output "" } */
/* { dg-begin-multiline-output "" }
FOR_EACH (i, 0, 10)
diff --git a/gcc/testsuite/c-c++-common/pr70264.c b/gcc/testsuite/c-c++-common/pr70264.c
index 815aad1..c446942 100644
--- a/gcc/testsuite/c-c++-common/pr70264.c
+++ b/gcc/testsuite/c-c++-common/pr70264.c
@@ -5,7 +5,7 @@ X
/* { dg-begin-multiline-output "" }
#define X __LINE__
- ^
+ ^~~~~~~~
{ dg-end-multiline-output "" } */
/* { dg-begin-multiline-output "" }
X
diff --git a/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C b/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C
index 4d3b07c..4efb9b3 100644
--- a/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C
+++ b/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C
@@ -782,3 +782,81 @@ tests::test_method_calls ()
~~~~~~~~~~~~~~~~~~^~
{ dg-end-multiline-output "" } */
}
+
+/* Various tests of locations involving macros. */
+
+void test_within_macro_1 (int lhs, int rhs)
+{
+#define MACRO_1(EXPR) EXPR
+
+ __emit_expression_range (0, MACRO_1 (lhs == rhs));
+
+/* { dg-warning "range" "" { target *-*-* } .-2 } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_1 (lhs == rhs));
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define MACRO_1(EXPR) EXPR
+ ^~~~
+ { dg-end-multiline-output "" } */
+
+#undef MACRO_1
+}
+
+void test_within_macro_2 (int lhs, int rhs)
+{
+#define MACRO_2(EXPR) EXPR
+
+ __emit_expression_range (0, MACRO_2 (MACRO_2 (lhs == rhs)));
+
+/* { dg-warning "range" "" { target *-*-* } .-2 } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_2 (MACRO_2 (lhs == rhs)));
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_2 (MACRO_2 (lhs == rhs)));
+ ^~~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define MACRO_2(EXPR) EXPR
+ ^~~~
+ { dg-end-multiline-output "" } */
+
+#undef MACRO_2
+}
+
+void test_within_macro_3 (int lhs, int rhs)
+{
+#define MACRO_3(EXPR) EXPR
+
+ __emit_expression_range (0, MACRO_3 (lhs) == MACRO_3 (rhs));
+
+/* { dg-warning "range" "" { target *-*-* } .-2 } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_3 (lhs) == MACRO_3 (rhs));
+ ^
+ { dg-end-multiline-output "" } */
+
+#undef MACRO_3
+}
+
+void test_within_macro_4 (int lhs, int rhs)
+{
+#define MACRO_4(EXPR) EXPR
+
+ __emit_expression_range (0, MACRO_4 (MACRO_4 (lhs) == MACRO_4 (rhs)));
+
+/* { dg-warning "range" "" { target *-*-* } .-2 } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_4 (MACRO_4 (lhs) == MACRO_4 (rhs)));
+ ^
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define MACRO_4(EXPR) EXPR
+ ^~~~
+ { dg-end-multiline-output "" } */
+
+#undef MACRO_4
+}
diff --git a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
index cab30f2..8cce5b3 100644
--- a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
+++ b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
@@ -278,7 +278,7 @@ void test_macro_3 (const char *msg)
printf(FMT_STRING, msg); /* { dg-message "10: in expansion of macro 'FMT_STRING" } */
/* { dg-begin-multiline-output "" }
#define FMT_STRING "hello %i world"
- ^
+ ^~~~~~~~~~~~~~~~
{ dg-end-multiline-output "" } */
/* { dg-begin-multiline-output "" }
printf(FMT_STRING, msg);
@@ -293,7 +293,7 @@ void test_macro_4 (const char *msg)
printf(FMT_STRING "\n", msg); /* { dg-message "10: in expansion of macro 'FMT_STRING" } */
/* { dg-begin-multiline-output "" }
#define FMT_STRING "hello %i world"
- ^
+ ^~~~~~~~~~~~~~~~
{ dg-end-multiline-output "" } */
/* { dg-begin-multiline-output "" }
printf(FMT_STRING "\n", msg);
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
index afbe0f7..523c32b 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
@@ -708,3 +708,82 @@ baz");
~~~~
{ dg-end-multiline-output "" } */
}
+
+/* Various tests of locations involving macros. */
+
+void test_within_macro_1 (int lhs, int rhs)
+{
+#define MACRO_1(EXPR) EXPR
+
+ __emit_expression_range (0, MACRO_1 (lhs == rhs));
+
+/* { dg-warning "range" "" { target *-*-* } .-2 } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_1 (lhs == rhs));
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define MACRO_1(EXPR) EXPR
+ ^~~~
+ { dg-end-multiline-output "" } */
+
+#undef MACRO_1
+}
+
+void test_within_macro_2 (int lhs, int rhs)
+{
+#define MACRO_2(EXPR) EXPR
+
+ __emit_expression_range (0, MACRO_2 (MACRO_2 (lhs == rhs)));
+
+/* { dg-warning "range" "" { target *-*-* } .-2 } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_2 (MACRO_2 (lhs == rhs)));
+ ~~~~^~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_2 (MACRO_2 (lhs == rhs)));
+ ^~~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define MACRO_2(EXPR) EXPR
+ ^~~~
+ { dg-end-multiline-output "" } */
+
+#undef MACRO_2
+}
+
+void test_within_macro_3 (int lhs, int rhs)
+{
+#define MACRO_3(EXPR) EXPR
+
+ __emit_expression_range (0, MACRO_3 (lhs) == MACRO_3 (rhs));
+
+/* { dg-warning "range" "" { target *-*-* } .-2 } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_3 (lhs) == MACRO_3 (rhs));
+ ^
+ { dg-end-multiline-output "" } */
+
+#undef MACRO_3
+}
+
+void test_within_macro_4 (int lhs, int rhs)
+{
+#define MACRO_4(EXPR) EXPR
+
+ __emit_expression_range (0, MACRO_4 (MACRO_4 (lhs) == MACRO_4 (rhs)));
+
+/* { dg-warning "range" "" { target *-*-* } .-2 } */
+/* { dg-begin-multiline-output "" }
+ __emit_expression_range (0, MACRO_4 (MACRO_4 (lhs) == MACRO_4 (rhs)));
+ ^
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define MACRO_4(EXPR) EXPR
+ ^~~~
+ { dg-end-multiline-output "" } */
+
+#undef MACRO_4
+}
+
diff --git a/gcc/testsuite/gcc.dg/spellcheck-fields-2.c b/gcc/testsuite/gcc.dg/spellcheck-fields-2.c
index 7c54214..76684f7 100644
--- a/gcc/testsuite/gcc.dg/spellcheck-fields-2.c
+++ b/gcc/testsuite/gcc.dg/spellcheck-fields-2.c
@@ -28,7 +28,7 @@ int test_macro (union u *ptr)
/* { dg-begin-multiline-output "" }
#define FIELD colour
- ^
+ ^~~~~~
{ dg-end-multiline-output "" } */
/* { dg-begin-multiline-output "" }
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index ec56de3..2ade65a 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -1903,6 +1903,15 @@ void linemap_dump (FILE *, struct line_maps *, unsigned, bool);
specifies how many macro maps to dump. */
void line_table_dump (FILE *, struct line_maps *, unsigned int, unsigned int);
+/* An enum for distinguishing the various parts within a source_location. */
+
+enum location_aspect
+{
+ LOCATION_ASPECT_CARET,
+ LOCATION_ASPECT_START,
+ LOCATION_ASPECT_FINISH
+};
+
/* The rich_location class requires a way to expand source_location instances.
We would directly use expand_location_to_spelling_point, which is
implemented in gcc/input.c, but we also need to use it for rich_location
@@ -1910,6 +1919,7 @@ void line_table_dump (FILE *, struct line_maps *, unsigned int, unsigned int);
Hence we require client code of libcpp to implement the following
symbol. */
extern expanded_location
-linemap_client_expand_location_to_spelling_point (source_location );
+linemap_client_expand_location_to_spelling_point (source_location,
+ enum location_aspect);
#endif /* !LIBCPP_LINE_MAP_H */
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 949489e..fb1c384 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -2011,13 +2011,15 @@ bool
source_range::intersects_line_p (const char *file, int line) const
{
expanded_location exploc_start
- = linemap_client_expand_location_to_spelling_point (m_start);
+ = linemap_client_expand_location_to_spelling_point (m_start,
+ LOCATION_ASPECT_START);
if (file != exploc_start.file)
return false;
if (line < exploc_start.line)
return false;
expanded_location exploc_finish
- = linemap_client_expand_location_to_spelling_point (m_finish);
+ = linemap_client_expand_location_to_spelling_point (m_finish,
+ LOCATION_ASPECT_FINISH);
if (file != exploc_finish.file)
return false;
if (line > exploc_finish.line)
@@ -2086,7 +2088,8 @@ rich_location::get_expanded_location (unsigned int idx)
if (!m_have_expanded_location)
{
m_expanded_location
- = linemap_client_expand_location_to_spelling_point (get_loc (0));
+ = linemap_client_expand_location_to_spelling_point
+ (get_loc (0), LOCATION_ASPECT_CARET);
if (m_column_override)
m_expanded_location.column = m_column_override;
m_have_expanded_location = true;
@@ -2095,7 +2098,8 @@ rich_location::get_expanded_location (unsigned int idx)
return m_expanded_location;
}
else
- return linemap_client_expand_location_to_spelling_point (get_loc (idx));
+ return linemap_client_expand_location_to_spelling_point
+ (get_loc (idx), LOCATION_ASPECT_CARET);
}
/* Set the column of the primary location, with 0 meaning
@@ -2421,7 +2425,8 @@ bool
fixit_insert::affects_line_p (const char *file, int line) const
{
expanded_location exploc
- = linemap_client_expand_location_to_spelling_point (m_where);
+ = linemap_client_expand_location_to_spelling_point (m_where,
+ LOCATION_ASPECT_CARET);
if (file == exploc.file)
if (line == exploc.line)
return true;
--
1.8.5.3
More information about the Gcc-patches
mailing list