From 4394b1ce7731b04446555e034163b14b4f4a7f8d Mon Sep 17 00:00:00 2001 From: Patrick Palka Date: Mon, 9 Nov 2020 11:09:42 -0500 Subject: [PATCH] c-family: Fix regression in location-overflow-test-1.c [PR97117] The r11-3266 patch that added macro support to -Wmisleading-indentation accidentally suppressed the column-tracking diagnostic in get_visual_column in some cases, e.g. in the location-overflow-test-1.c testcase. More generally, when all three tokens are on the same line and we've run out of locations with column info, then their location_t values will be equal, and we exit early from should_warn_for_misleading_indentation due to the new check /* Give up if the loci are not all distinct. */ if (guard_loc == body_loc || body_loc == next_stmt_loc) return false; before we ever call get_visual_column. [ This new check is needed to detect and give up on analyzing code fragments where exactly two out of the three tokens come from the same macro expansion, e.g. #define MACRO \ if (a) \ foo (); MACRO; bar (); Here, guard_loc and body_loc will be equal and point to the macro expansion point (and next_stmt_loc will point to 'bar'). The heuristics that the warning uses are not really valid in scenarios like these. ] In order to restore the column-tracking diagnostic, this patch moves the the diagnostic code out from get_visual_column to earlier in should_warn_for_misleading_indentation. Moreover, it tests the three locations for a zero column all at once, which I suppose should make us issue the diagnostic more consistently. gcc/c-family/ChangeLog: PR testsuite/97117 * c-indentation.c (get_visual_column): Remove location_t parameter. Move the column-tracking diagnostic code from here to ... (should_warn_for_misleading_indentation): ... here, before the early exit for when the loci are not all distinct. Don't pass a location_t argument to get_visual_column. (assert_get_visual_column_succeeds): Don't pass a location_t argument to get_visual_column. (assert_get_visual_column_fails): Likewise. --- gcc/c-family/c-indentation.c | 70 +++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c index 8b88a8a..836a524 100644 --- a/gcc/c-family/c-indentation.c +++ b/gcc/c-family/c-indentation.c @@ -45,36 +45,11 @@ next_tab_stop (unsigned int vis_column, unsigned int tab_width) on the line (up to or before EXPLOC). */ static bool -get_visual_column (expanded_location exploc, location_t loc, +get_visual_column (expanded_location exploc, unsigned int *out, unsigned int *first_nws, unsigned int tab_width) { - /* PR c++/68819: if the column number is zero, we presumably - had a location_t > LINE_MAP_MAX_LOCATION_WITH_COLS, and so - we have no column information. - Act as if no conversion was possible, triggering the - error-handling path in the caller. */ - if (!exploc.column) - { - static bool issued_note = false; - if (!issued_note) - { - /* Notify the user the first time this happens. */ - issued_note = true; - inform (loc, - "%<-Wmisleading-indentation%> is disabled from this point" - " onwards, since column-tracking was disabled due to" - " the size of the code/headers"); - if (!flag_large_source_files) - inform (loc, - "adding %<-flarge-source-files%> will allow for more" - " column-tracking support, at the expense of compilation" - " time and memory"); - } - return false; - } - char_span line = location_get_source_line (exploc.file, exploc.line); if (!line) return false; @@ -325,14 +300,37 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, NULL); } - /* Give up if the loci are not all distinct. */ - if (guard_loc == body_loc || body_loc == next_stmt_loc) - return false; - expanded_location body_exploc = expand_location (body_loc); expanded_location next_stmt_exploc = expand_location (next_stmt_loc); expanded_location guard_exploc = expand_location (guard_loc); + /* PR c++/68819: if the column number is zero, we presumably + had a location_t > LINE_MAP_MAX_LOCATION_WITH_COLS, and so + we have no column information. */ + if (!guard_exploc.column || !body_exploc.column || !next_stmt_exploc.column) + { + static bool issued_note = false; + if (!issued_note) + { + /* Notify the user the first time this happens. */ + issued_note = true; + inform (guard_loc, + "%<-Wmisleading-indentation%> is disabled from this point" + " onwards, since column-tracking was disabled due to" + " the size of the code/headers"); + if (!flag_large_source_files) + inform (guard_loc, + "adding %<-flarge-source-files%> will allow for more" + " column-tracking support, at the expense of compilation" + " time and memory"); + } + return false; + } + + /* Give up if the loci are not all distinct. */ + if (guard_loc == body_loc || body_loc == next_stmt_loc) + return false; + const unsigned int tab_width = global_dc->tabstop; /* They must be in the same file. */ @@ -378,7 +376,7 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, gcc_assert (guard_exploc.line == next_stmt_exploc.line); unsigned int guard_vis_column; unsigned int guard_line_first_nws; - if (!get_visual_column (guard_exploc, guard_loc, + if (!get_visual_column (guard_exploc, &guard_vis_column, &guard_line_first_nws, tab_width)) return false; @@ -438,15 +436,15 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, the case for input files containing #line directives, and these are often for autogenerated sources (e.g. from .md files), where it's not clear that it's meaningful to look at indentation. */ - if (!get_visual_column (next_stmt_exploc, next_stmt_loc, + if (!get_visual_column (next_stmt_exploc, &next_stmt_vis_column, &next_stmt_line_first_nws, tab_width)) return false; - if (!get_visual_column (body_exploc, body_loc, + if (!get_visual_column (body_exploc, &body_vis_column, &body_line_first_nws, tab_width)) return false; - if (!get_visual_column (guard_exploc, guard_loc, + if (!get_visual_column (guard_exploc, &guard_vis_column, &guard_line_first_nws, tab_width)) return false; @@ -701,7 +699,7 @@ assert_get_visual_column_succeeds (const location &loc, exploc.sysp = false; unsigned int actual_visual_column; unsigned int actual_first_nws; - bool result = get_visual_column (exploc, UNKNOWN_LOCATION, + bool result = get_visual_column (exploc, &actual_visual_column, &actual_first_nws, tab_width); ASSERT_TRUE_AT (loc, result); @@ -739,7 +737,7 @@ assert_get_visual_column_fails (const location &loc, exploc.sysp = false; unsigned int actual_visual_column; unsigned int actual_first_nws; - bool result = get_visual_column (exploc, UNKNOWN_LOCATION, + bool result = get_visual_column (exploc, &actual_visual_column, &actual_first_nws, tab_width); ASSERT_FALSE_AT (loc, result); -- 2.7.4