On Sun, Nov 3, 2024 at 11:28 PM Lewis Hyatt <lhy...@gmail.com> wrote:
>
> Several of the selftests in diagnostic-show-locus.cc and input.cc are
> sensitive to linemap internals. Adjust them here so they will support 64-bit
> location_t if configured.
>
> Likewise, handle 64-bit location_t in the support for
> -fdump-internal-locations. As was done with the analyzer, convert to
> (unsigned long long) explicitly so that 32- and 64-bit can be handled with
> the same printf formats.
I was hoping David would have a look here. Absent from comments from him
this is OK when all else is approved and after giving him another week.
What's missing review now? I've lost track ...
Thanks,
Richard.
> gcc/ChangeLog:
>
> * diagnostic-show-locus.cc
> (test_one_liner_fixit_validation_adhoc_locations): Adapt so it can
> effectively test 7-bit ranges instead of 5-bit ranges.
> (test_one_liner_fixit_validation_adhoc_locations_utf8): Likewise.
> * input.cc (get_end_location): Adjust types to support 64-bit
> location_t.
> (write_digit_row): Likewise.
> (dump_location_range): Likewise.
> (dump_location_info): Likewise.
> (class line_table_case): Likewise.
> (test_accessing_ordinary_linemaps): Replace some hard-coded
> constants with the values defined in line-map.h.
> (for_each_line_table_case): Likewise.
> ---
> gcc/diagnostic-show-locus.cc | 128 +++++++++++++++++++++++++++++------
> gcc/input.cc | 100 ++++++++++++++-------------
> 2 files changed, 157 insertions(+), 71 deletions(-)
>
> diff --git a/gcc/diagnostic-show-locus.cc b/gcc/diagnostic-show-locus.cc
> index a2a4b047ff9..ffe9e807104 100644
> --- a/gcc/diagnostic-show-locus.cc
> +++ b/gcc/diagnostic-show-locus.cc
> @@ -4013,12 +4013,12 @@ test_one_liner_fixit_validation_adhoc_locations ()
> {
> /* Generate a range that's too long to be packed, so must
> be stored as an ad-hoc location (given the defaults
> - of 5 bits or 0 bits of packed range); 41 columns > 2**5. */
> + of 5 or 7 bits or 0 bits of packed range); 150 columns > 2**7. */
> const location_t c7 = linemap_position_for_column (line_table, 7);
> - const location_t c47 = linemap_position_for_column (line_table, 47);
> - const location_t loc = make_location (c7, c7, c47);
> + const location_t c157 = linemap_position_for_column (line_table, 157);
> + const location_t loc = make_location (c7, c7, c157);
>
> - if (c47 > LINE_MAP_MAX_LOCATION_WITH_COLS)
> + if (c157 > LINE_MAP_MAX_LOCATION_WITH_COLS)
> return;
>
> ASSERT_TRUE (IS_ADHOC_LOC (loc));
> @@ -4032,7 +4032,18 @@ test_one_liner_fixit_validation_adhoc_locations ()
>
> test_diagnostic_context dc;
> ASSERT_STREQ (" foo = bar.field;\n"
> - " ^~~~~~~~~~ \n"
> + " ^~~~~~~~~~ "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " \n"
> " test\n",
> dc.test_show_locus (richloc));
> }
> @@ -4040,29 +4051,62 @@ test_one_liner_fixit_validation_adhoc_locations ()
> /* Remove. */
> {
> rich_location richloc (line_table, loc);
> - source_range range = source_range::from_locations (loc, c47);
> + source_range range = source_range::from_locations (loc, c157);
> richloc.add_fixit_remove (range);
> /* It should not have been discarded by the validator. */
> ASSERT_EQ (1, richloc.get_num_fixit_hints ());
>
> test_diagnostic_context dc;
> ASSERT_STREQ (" foo = bar.field;\n"
> - " ^~~~~~~~~~ \n"
> - " -----------------------------------------\n",
> + " ^~~~~~~~~~ "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " \n"
> + " -----------------------------------------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------\n",
> dc.test_show_locus (richloc));
> }
>
> /* Replace. */
> {
> rich_location richloc (line_table, loc);
> - source_range range = source_range::from_locations (loc, c47);
> + source_range range = source_range::from_locations (loc, c157);
> richloc.add_fixit_replace (range, "test");
> /* It should not have been discarded by the validator. */
> ASSERT_EQ (1, richloc.get_num_fixit_hints ());
>
> test_diagnostic_context dc;
> ASSERT_STREQ (" foo = bar.field;\n"
> - " ^~~~~~~~~~ \n"
> + " ^~~~~~~~~~ "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " \n"
> " test\n",
> dc.test_show_locus (richloc));
> }
> @@ -4552,12 +4596,12 @@ test_one_liner_fixit_validation_adhoc_locations_utf8
> ()
> {
> /* Generate a range that's too long to be packed, so must
> be stored as an ad-hoc location (given the defaults
> - of 5 bits or 0 bits of packed range); 41 columns > 2**5. */
> + of 5 bits or 7 bits or 0 bits of packed range); 150 columns > 2**7. */
> const location_t c12 = linemap_position_for_column (line_table, 12);
> - const location_t c52 = linemap_position_for_column (line_table, 52);
> - const location_t loc = make_location (c12, c12, c52);
> + const location_t c162 = linemap_position_for_column (line_table, 162);
> + const location_t loc = make_location (c12, c12, c162);
>
> - if (c52 > LINE_MAP_MAX_LOCATION_WITH_COLS)
> + if (c162 > LINE_MAP_MAX_LOCATION_WITH_COLS)
> return;
>
> ASSERT_TRUE (IS_ADHOC_LOC (loc));
> @@ -4575,7 +4619,18 @@ test_one_liner_fixit_validation_adhoc_locations_utf8 ()
> "_bar.\xf0\x9f\x98\x82"
> "_field\xcf\x80"
> ";\n"
> - " ^~~~~~~~~~~~~~~~ \n"
> + " ^~~~~~~~~~~~~~~~ "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " \n"
> " test\n",
> dc.test_show_locus (richloc));
> }
> @@ -4583,7 +4638,7 @@ test_one_liner_fixit_validation_adhoc_locations_utf8 ()
> /* Remove. */
> {
> rich_location richloc (line_table, loc);
> - source_range range = source_range::from_locations (loc, c52);
> + source_range range = source_range::from_locations (loc, c162);
> richloc.add_fixit_remove (range);
> /* It should not have been discarded by the validator. */
> ASSERT_EQ (1, richloc.get_num_fixit_hints ());
> @@ -4594,15 +4649,37 @@ test_one_liner_fixit_validation_adhoc_locations_utf8
> ()
> "_bar.\xf0\x9f\x98\x82"
> "_field\xcf\x80"
> ";\n"
> - " ^~~~~~~~~~~~~~~~ \n"
> - " -------------------------------------\n",
> + " ^~~~~~~~~~~~~~~~ "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " \n"
> + " -------------------------------------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------"
> + "----------\n",
> dc.test_show_locus (richloc));
> }
>
> /* Replace. */
> {
> rich_location richloc (line_table, loc);
> - source_range range = source_range::from_locations (loc, c52);
> + source_range range = source_range::from_locations (loc, c162);
> richloc.add_fixit_replace (range, "test");
> /* It should not have been discarded by the validator. */
> ASSERT_EQ (1, richloc.get_num_fixit_hints ());
> @@ -4613,7 +4690,18 @@ test_one_liner_fixit_validation_adhoc_locations_utf8 ()
> "_bar.\xf0\x9f\x98\x82"
> "_field\xcf\x80"
> ";\n"
> - " ^~~~~~~~~~~~~~~~ \n"
> + " ^~~~~~~~~~~~~~~~ "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " "
> + " \n"
> " test\n",
> dc.test_show_locus (richloc));
> }
> diff --git a/gcc/input.cc b/gcc/input.cc
> index 04462ef6f5a..1629e4aeee8 100644
> --- a/gcc/input.cc
> +++ b/gcc/input.cc
> @@ -1271,7 +1271,7 @@ dump_line_table_statistics (void)
> /* Get location one beyond the final location in ordinary map IDX. */
>
> static location_t
> -get_end_location (class line_maps *set, unsigned int idx)
> +get_end_location (class line_maps *set, line_map_uint_t idx)
> {
> if (idx == LINEMAPS_ORDINARY_USED (set) - 1)
> return set->highest_location;
> @@ -1301,7 +1301,7 @@ write_digit_row (FILE *stream, int indent,
> fprintf (stream, "|");
> for (int column = 1; column < max_col; column++)
> {
> - location_t column_loc = loc + (column << map->m_range_bits);
> + location_t column_loc = loc + (location_t (column) <<
> map->m_range_bits);
> write_digit (stream, column_loc / divisor);
> }
> fprintf (stream, "\n");
> @@ -1315,8 +1315,8 @@ dump_location_range (FILE *stream,
> location_t start, location_t end)
> {
> fprintf (stream,
> - " location_t interval: %u <= loc < %u\n",
> - start, end);
> + " location_t interval: %llu <= loc < %llu\n",
> + (unsigned long long) start, (unsigned long long) end);
> }
>
> /* Write a labelled description of a half-closed (START) / half-open (END)
> @@ -1343,15 +1343,18 @@ dump_location_info (FILE *stream)
> dump_labelled_location_range (stream, "RESERVED LOCATIONS",
> 0, RESERVED_LOCATION_COUNT);
>
> + using ULL = unsigned long long;
> +
> /* Visualize the ordinary line_map instances, rendering the sources. */
> - for (unsigned int idx = 0; idx < LINEMAPS_ORDINARY_USED (line_table);
> idx++)
> + for (line_map_uint_t idx = 0; idx < LINEMAPS_ORDINARY_USED (line_table);
> + idx++)
> {
> location_t end_location = get_end_location (line_table, idx);
> /* half-closed: doesn't include this one. */
>
> const line_map_ordinary *map
> = LINEMAPS_ORDINARY_MAP_AT (line_table, idx);
> - fprintf (stream, "ORDINARY MAP: %i\n", idx);
> + fprintf (stream, "ORDINARY MAP: %llu\n", (ULL) idx);
> dump_location_range (stream,
> MAP_START_LOCATION (map), end_location);
> fprintf (stream, " file: %s\n", ORDINARY_MAP_FILE_NAME (map));
> @@ -1387,18 +1390,18 @@ dump_location_info (FILE *stream)
>
> const line_map_ordinary *includer_map
> = linemap_included_from_linemap (line_table, map);
> - fprintf (stream, " included from location: %d",
> - linemap_included_from (map));
> + fprintf (stream, " included from location: %llu",
> + (ULL) linemap_included_from (map));
> if (includer_map) {
> - fprintf (stream, " (in ordinary map %d)",
> - int (includer_map - line_table->info_ordinary.maps));
> + fprintf (stream, " (in ordinary map %llu)",
> + ULL (includer_map - line_table->info_ordinary.maps));
> }
> fprintf (stream, "\n");
>
> /* Render the span of source lines that this "map" covers. */
> for (location_t loc = MAP_START_LOCATION (map);
> loc < end_location;
> - loc += (1 << map->m_range_bits) )
> + loc += (location_t (1) << map->m_range_bits))
> {
> gcc_assert (pure_location_p (line_table, loc) );
>
> @@ -1414,16 +1417,16 @@ dump_location_info (FILE *stream)
> if (!line_text)
> break;
> fprintf (stream,
> - "%s:%3i|loc:%5i|%.*s\n",
> + "%s:%3i|loc:%5llu|%.*s\n",
> exploc.file, exploc.line,
> - loc,
> + (ULL) loc,
> (int)line_text.length (), line_text.get_buffer ());
>
> /* "loc" is at column 0, which means "the whole line".
> Render the locations *within* the line, by underlining
> it, showing the location_t numeric values
> at each column. */
> - size_t max_col = (1 << map->m_column_and_range_bits) - 1;
> + auto max_col = (ULL (1) << map->m_column_and_range_bits) - 1;
> if (max_col > line_text.length ())
> max_col = line_text.length () + 1;
>
> @@ -1460,19 +1463,19 @@ dump_location_info (FILE *stream)
> LINEMAPS_MACRO_LOWEST_LOCATION (line_table));
>
> /* Visualize the macro line_map instances, rendering the sources. */
> - for (unsigned int i = 0; i < LINEMAPS_MACRO_USED (line_table); i++)
> + for (line_map_uint_t i = 0; i < LINEMAPS_MACRO_USED (line_table); i++)
> {
> /* Each macro map that is allocated owns location_t values
> that are *lower* that the one before them.
> Hence it's meaningful to view them either in order of ascending
> source locations, or in order of ascending macro map index. */
> const bool ascending_location_ts = true;
> - unsigned int idx = (ascending_location_ts
> - ? (LINEMAPS_MACRO_USED (line_table) - (i + 1))
> - : i);
> + auto idx = (ascending_location_ts
> + ? (LINEMAPS_MACRO_USED (line_table) - (i + 1))
> + : i);
> const line_map_macro *map = LINEMAPS_MACRO_MAP_AT (line_table, idx);
> - fprintf (stream, "MACRO %i: %s (%u tokens)\n",
> - idx,
> + fprintf (stream, "MACRO %llu: %s (%u tokens)\n",
> + (ULL) idx,
> linemap_map_get_macro_name (map),
> MACRO_MAP_NUM_MACRO_TOKENS (map));
> dump_location_range (stream,
> @@ -1480,10 +1483,10 @@ dump_location_info (FILE *stream)
> (map->start_location
> + MACRO_MAP_NUM_MACRO_TOKENS (map)));
> inform (map->get_expansion_point_location (),
> - "expansion point is location %i",
> - map->get_expansion_point_location ());
> - fprintf (stream, " map->start_location: %u\n",
> - map->start_location);
> + "expansion point is location %llu",
> + (ULL) map->get_expansion_point_location ());
> + fprintf (stream, " map->start_location: %llu\n",
> + (ULL) map->start_location);
>
> fprintf (stream, " macro_locations:\n");
> for (unsigned int i = 0; i < MACRO_MAP_NUM_MACRO_TOKENS (map); i++)
> @@ -1503,24 +1506,26 @@ dump_location_info (FILE *stream)
> This would explain there being up to 4 location_ts slots
> that may be uninitialized. */
>
> - fprintf (stream, " %u: %u, %u\n",
> + fprintf (stream, " %u: %llu, %llu\n",
> i,
> - x,
> - y);
> + (ULL) x,
> + (ULL) y);
> if (x == y)
> {
> if (x < MAP_START_LOCATION (map))
> - inform (x, "token %u has %<x-location == y-location == %u%>",
> - i, x);
> + inform (x, "token %u has %<x-location == y-location ==
> %llu%>",
> + i, (ULL) x);
> else
> fprintf (stream,
> - "x-location == y-location == %u encodes token #
> %u\n",
> - x, x - MAP_START_LOCATION (map));
> - }
> + "x-location == y-location == %llu"
> + " encodes token # %u\n",
> + (ULL) x,
> + (unsigned int)(x - MAP_START_LOCATION (map)));
> + }
> else
> {
> - inform (x, "token %u has %<x-location == %u%>", i, x);
> - inform (x, "token %u has %<y-location == %u%>", i, y);
> + inform (x, "token %u has %<x-location == %llu%>", i, (ULL) x);
> + inform (x, "token %u has %<y-location == %llu%>", i, (ULL) y);
> }
> }
> fprintf (stream, "\n");
> @@ -1536,7 +1541,7 @@ dump_location_info (FILE *stream)
>
> /* Visualize ad-hoc values. */
> dump_labelled_location_range (stream, "AD-HOC LOCATIONS",
> - MAX_LOCATION_T + 1, UINT_MAX);
> + MAX_LOCATION_T + 1, location_t (-1));
> }
>
> /* string_concat's constructor. */
> @@ -2036,13 +2041,13 @@ assert_loceq (const char *exp_filename, int
> exp_linenum, int exp_colnum,
> class line_table_case
> {
> public:
> - line_table_case (int default_range_bits, int base_location)
> + line_table_case (int default_range_bits, location_t base_location)
> : m_default_range_bits (default_range_bits),
> m_base_location (base_location)
> {}
>
> int m_default_range_bits;
> - int m_base_location;
> + location_t m_base_location;
> };
>
> /* Constructor. Store the old value of line_table, and create a new
> @@ -2132,9 +2137,9 @@ test_accessing_ordinary_linemaps (const line_table_case
> &case_)
> location_t loc_start_of_very_long_line
> = linemap_position_for_column (line_table, 2000);
> location_t loc_too_wide
> - = linemap_position_for_column (line_table, 4097);
> + = linemap_position_for_column (line_table, LINE_MAP_MAX_COLUMN_NUMBER +
> 1);
> location_t loc_too_wide_2
> - = linemap_position_for_column (line_table, 4098);
> + = linemap_position_for_column (line_table, LINE_MAP_MAX_COLUMN_NUMBER +
> 2);
>
> /* ...and back to a sane line length. */
> linemap_line_start (line_table, 6, 100);
> @@ -3865,11 +3870,11 @@ static const location_t boundary_locations[] = {
> LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES + 0x100,
>
> /* Values near LINE_MAP_MAX_LOCATION_WITH_COLS. */
> - LINE_MAP_MAX_LOCATION_WITH_COLS - 0x100,
> + LINE_MAP_MAX_LOCATION_WITH_COLS - 0x200,
> LINE_MAP_MAX_LOCATION_WITH_COLS - 1,
> LINE_MAP_MAX_LOCATION_WITH_COLS,
> LINE_MAP_MAX_LOCATION_WITH_COLS + 1,
> - LINE_MAP_MAX_LOCATION_WITH_COLS + 0x100,
> + LINE_MAP_MAX_LOCATION_WITH_COLS + 0x200,
> };
>
> /* Run TESTCASE multiple times, once for each case in our test matrix. */
> @@ -3884,10 +3889,9 @@ for_each_line_table_case (void (*testcase) (const
> line_table_case &))
>
> /* Run all tests with:
> (a) line_table->default_range_bits == 0, and
> - (b) line_table->default_range_bits == 5. */
> - int num_cases_tested = 0;
> - for (int default_range_bits = 0; default_range_bits <= 5;
> - default_range_bits += 5)
> + (b) line_table->default_range_bits == line_map_suggested_range_bits. */
> +
> + for (int default_range_bits: {0, line_map_suggested_range_bits})
> {
> /* ...and use each of the "interesting" location values as
> the starting location within line_table. */
> @@ -3895,15 +3899,9 @@ for_each_line_table_case (void (*testcase) (const
> line_table_case &))
> for (int loc_idx = 0; loc_idx < num_boundary_locations; loc_idx++)
> {
> line_table_case c (default_range_bits, boundary_locations[loc_idx]);
> -
> testcase (c);
> -
> - num_cases_tested++;
> }
> }
> -
> - /* Verify that we fully covered the test matrix. */
> - ASSERT_EQ (num_cases_tested, 2 * 12);
> }
>
> /* Verify that when presented with a consecutive pair of locations with
