Ping. I believe I need review of the selftest.h change; the rest I think I can self-approve, if need be.
https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01340.html On Fri, 2016-06-17 at 17:41 -0400, David Malcolm wrote: > This patch adds explicit testing of lexing a source file, > generalizing this (and the test of ordinary line maps) over > a 2-dimensional test matrix covering: > > (1) line_table->default_range_bits: some frontends use a non-zero value > and others use zero > > (2) the fallback modes within line-map.c: there are various threshold > values for source_location/location_t beyond line-map.c changes > behavior (disabling of the range-packing optimization, disabling > of column-tracking). We exercise these by starting the line_table > at interesting values at or near these thresholds. > > This helps ensures that location data works in all of these states, > and that (I hope) we don't have lingering bugs relating to the > transition between line_table states. > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu; > Successful -fself-test of stage1 on powerpc-ibm-aix7.1.3.0. > > OK for trunk? (I can self-approve much of this, but it's probably > worth having another pair of eyes look at it, if nothing else). > > gcc/ChangeLog: > > * input.c: Include cpplib.h. > > (selftest::temp_source_file): New class. > > (selftest::temp_source_file::temp_source_file): New ctor. > > (selftest::temp_source_file::~temp_source_file): New dtor. > > (selftest::should_have_column_data_p): New function. > > (selftest::test_should_have_column_data_p): New function. > > (selftest::temp_line_table): New class. > > (selftest::temp_line_table::temp_line_table): New ctor. > > (selftest::temp_line_table::~temp_line_table): New dtor. > > (selftest::test_accessing_ordinary_linemaps): Add case_ param; use > > it to create a temp_line_table. > > (selftest::assert_loceq): Only verify LOCATION_COLUMN for > > locations that are known to have column data. > > (selftest::line_table_case): New struct. > > (selftest::test_reading_source_line): Move tempfile handling > > to class temp_source_file. > > (ASSERT_TOKEN_AS_TEXT_EQ): New macro. > > (selftest::assert_token_loc_eq): New function. > > (ASSERT_TOKEN_LOC_EQ): New macro. > > (selftest::test_lexer): New function. > > (selftest::boundary_locations): New array. > > (selftest::input_c_tests): Call test_should_have_column_data_p. > > Loop over a test matrix of interesting values of location and > > default_range_bits, calling test_lexer on each case in the matrix. > > Move call to test_accessing_ordinary_linemaps into the matrix. > > * selftest.h (ASSERT_EQ): Reimplement in terms of... > > (ASSERT_EQ_AT): New macro. > > gcc/testsuite/ChangeLog: > > * gcc.dg/plugin/location_overflow_plugin.c (plugin_init): Avoid > > hardcoding the values of LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES > > and LINE_MAP_MAX_LOCATION_WITH_COLS. > > libcpp/ChangeLog: > > * include/line-map.h (LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES): > > Move here from line-map.c. > > (LINE_MAP_MAX_LOCATION_WITH_COLS): Likewise. > > * line-map.c (LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES): Move from > > here to line-map.h. > > (LINE_MAP_MAX_LOCATION_WITH_COLS): Likewise. > --- > gcc/input.c | 323 > +++++++++++++++++++-- > gcc/selftest.h | 12 +- > .../gcc.dg/plugin/location_overflow_plugin.c | 4 +- > libcpp/include/line-map.h | 10 + > libcpp/line-map.c | 12 - > 5 files changed, 327 insertions(+), 34 deletions(-) > > diff --git a/gcc/input.c b/gcc/input.c > index 3fb4a25..0016555 100644 > --- a/gcc/input.c > +++ b/gcc/input.c > @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see > #include "intl.h" > #include "diagnostic-core.h" > #include "selftest.h" > +#include "cpplib.h" > > /* This is a cache used by get_next_line to store the content of a > file to be searched for file lines. */ > @@ -1144,6 +1145,74 @@ namespace selftest { > > /* Selftests of location handling. */ > > +/* A class for writing out a temporary sourcefile for use in selftests > + of input handling. */ > + > +class temp_source_file > +{ > + public: > + temp_source_file (const location &loc, const char *suffix, > +> > > const char *content); > + ~temp_source_file (); > + > + const char *get_filename () const { return m_filename; } > + > + private: > + char *m_filename; > +}; > + > +/* Constructor. Create a tempfile using SUFFIX, and write CONTENT to > + it. Abort if anything goes wrong, using LOC as the effective > + location in the problem report. */ > + > +temp_source_file::temp_source_file (const location &loc, const char *suffix, > +> > > > > const char *content) > +{ > + m_filename = make_temp_file (suffix); > + ASSERT_NE (m_filename, NULL); > + > + FILE *out = fopen (m_filename, "w"); > + if (!out) > + ::selftest::fail_formatted (loc, "unable to open tempfile: %s", > +> > > > > m_filename); > + fprintf (out, content); > + fclose (out); > +} > + > +/* Destructor. Delete the tempfile. */ > + > +temp_source_file::~temp_source_file () > +{ > + unlink (m_filename); > + free (m_filename); > +} > + > +/* Helper function for verifying location data: when location_t > + values are > LINE_MAP_MAX_LOCATION_WITH_COLS, they are treated > + as having column 0. */ > + > +static bool > +should_have_column_data_p (location_t loc) > +{ > + if (IS_ADHOC_LOC (loc)) > + loc = get_location_from_adhoc_loc (line_table, loc); > + if (loc > LINE_MAP_MAX_LOCATION_WITH_COLS) > + return false; > + return true; > +} > + > +/* Selftest for should_have_column_data_p. */ > + > +static void > +test_should_have_column_data_p () > +{ > + ASSERT_TRUE (should_have_column_data_p (RESERVED_LOCATION_COUNT)); > + ASSERT_TRUE > + (should_have_column_data_p (LINE_MAP_MAX_LOCATION_WITH_COLS)); > + ASSERT_FALSE > + (should_have_column_data_p (LINE_MAP_MAX_LOCATION_WITH_COLS + 1)); > +} > + > /* Verify the result of LOCATION_FILE/LOCATION_LINE/LOCATION_COLUMN > on LOC. */ > > @@ -1153,14 +1222,87 @@ assert_loceq (const char *exp_filename, int > exp_linenum, int exp_colnum, > { > ASSERT_STREQ (exp_filename, LOCATION_FILE (loc)); > ASSERT_EQ (exp_linenum, LOCATION_LINE (loc)); > - ASSERT_EQ (exp_colnum, LOCATION_COLUMN (loc)); > + /* If location_t values are sufficiently high, then column numbers > + will be unavailable and LOCATION_COLUMN (loc) will be 0. > + When close to the threshold, column numbers *may* be present: if > + the final linemap before the threshold contains a line that straddles > + the threshold, locations in that line have column information. */ > + if (should_have_column_data_p (loc)) > + ASSERT_EQ (exp_colnum, LOCATION_COLUMN (loc)); > +} > + > +/* Various selftests in this file involve constructing a line table > + and one or more line maps within it. > + > + For maximum test coverage we want to run these tests with a variety > + of situations: > + - line_table->default_range_bits: some frontends use a non-zero value > + and others use zero > + - the fallback modes within line-map.c: there are various threshold > + values for source_location/location_t beyond line-map.c changes > + behavior (disabling of the range-packing optimization, disabling > + of column-tracking). We can exercise these by starting the line_table > + at interesting values at or near these thresholds. > + > + The following struct describes a particular case within our test > + matrix. */ > + > +struct line_table_case > +{ > + line_table_case (int default_range_bits, int base_location) > + : m_default_range_bits (default_range_bits), > + m_base_location (base_location) > + {} > + > + int m_default_range_bits; > + int m_base_location; > +}; > + > +/* A class for overriding the global "line_table" within a selftest, > + restoring its value afterwards. */ > + > +class temp_line_table > +{ > + public: > + temp_line_table (const line_table_case &); > + ~temp_line_table (); > + > + private: > + line_maps *m_old_line_table; > +}; > + > +/* Constructor. Store the old value of line_table, and create a new > + one, using the sitation described in CASE_. */ > + > +temp_line_table::temp_line_table (const line_table_case &case_) > + : m_old_line_table (line_table) > +{ > + line_table = ggc_alloc (); > + linemap_init (line_table, BUILTINS_LOCATION); > + line_table->reallocator = m_old_line_table->reallocator; > + line_table->round_alloc_size = m_old_line_table->round_alloc_size; > + line_table->default_range_bits = case_.m_default_range_bits; > + if (case_.m_base_location) > + { > + line_table->highest_location = case_.m_base_location; > + line_table->highest_line = case_.m_base_location; > + } > +} > + > +/* Destructor. Restore the old value of line_table. */ > + > +temp_line_table::~temp_line_table () > +{ > + line_table = m_old_line_table; > } > > /* Verify basic operation of ordinary linemaps. */ > > static void > -test_accessing_ordinary_linemaps () > +test_accessing_ordinary_linemaps (const line_table_case &case_) > { > + temp_line_table tmp_lt (case_); > + > /* Build a simple linemap describing some locations. */ > linemap_add (line_table, LC_ENTER, false, "foo.c", 0); > > @@ -1220,21 +1362,15 @@ static void > test_reading_source_line () > { > /* Create a tempfile and write some text to it. */ > - char *filename = make_temp_file (".txt"); > - ASSERT_NE (filename, NULL); > - FILE *out = fopen (filename, "w"); > - if (!out) > - ::selftest::fail_formatted (SELFTEST_LOCATION, > -> > > > > "unable to open tempfile: %s", filename); > - fprintf (out, > -> > "01234567890123456789\n" > -> > "This is the test text\n" > -> > "This is the 3rd line\n"); > - fclose (out); > + temp_source_file tmp (SELFTEST_LOCATION, ".txt", > +> > > > "01234567890123456789\n" > +> > > > "This is the test text\n" > +> > > > "This is the 3rd line\n"); > > /* Read back a specific line from the tempfile. */ > int line_size; > - const char *source_line = location_get_source_line (filename, 2, > &line_size); > + const char *source_line = location_get_source_line (tmp.get_filename (), > +> > > > > > > 2, &line_size); > ASSERT_TRUE (source_line != NULL); > ASSERT_EQ (21, line_size); > if (!strncmp ("This is the test text", > @@ -1245,18 +1381,171 @@ test_reading_source_line () > ::selftest::fail (SELFTEST_LOCATION, > > > > "source_line did not match expected value"); > > - unlink (filename); > - free (filename); > } > > +/* Tests of lexing. */ > + > +/* Verify that token TOK from PARSER has cpp_token_as_text > + equal to EXPECTED_TEXT. */ > + > +#define ASSERT_TOKEN_AS_TEXT_EQ(PARSER, TOK, EXPECTED_TEXT)> > > > \ > + SELFTEST_BEGIN_STMT> > > > > > > > > \ > + unsigned char *actual_txt = cpp_token_as_text ((PARSER), (TOK));> > > \ > + ASSERT_STREQ ((EXPECTED_TEXT), (const char *)actual_txt);> > > > \ > + SELFTEST_END_STMT > + > +/* Verify that TOK's src_loc is within EXP_FILENAME at EXP_LINENUM, > + and ranges from EXP_START_COL to EXP_FINISH_COL. > + Use LOC as the effective location of the selftest. */ > + > +static void > +assert_token_loc_eq (const location &loc, > +> > > const cpp_token *tok, > +> > > const char *exp_filename, int exp_linenum, > +> > > int exp_start_col, int exp_finish_col) > +{ > + location_t tok_loc = tok->src_loc; > + ASSERT_STREQ_AT (loc, exp_filename, LOCATION_FILE (tok_loc)); > + ASSERT_EQ_AT (loc, exp_linenum, LOCATION_LINE (tok_loc)); > + > + /* If location_t values are sufficiently high, then column numbers > + will be unavailable. */ > + if (!should_have_column_data_p (tok_loc)) > + return; > + > + ASSERT_EQ_AT (loc, exp_start_col, LOCATION_COLUMN (tok_loc)); > + source_range tok_range = get_range_from_loc (line_table, tok_loc); > + ASSERT_EQ_AT (loc, exp_start_col, LOCATION_COLUMN (tok_range.m_start)); > + ASSERT_EQ_AT (loc, exp_finish_col, LOCATION_COLUMN (tok_range.m_finish)); > +} > + > +/* Use assert_token_loc_eq to verify the TOK->src_loc, using > + SELFTEST_LOCATION as the effective location of the selftest. */ > + > +#define ASSERT_TOKEN_LOC_EQ(TOK, EXP_FILENAME, EXP_LINENUM, \ > +> > > > EXP_START_COL, EXP_FINISH_COL) \ > + assert_token_loc_eq (SELFTEST_LOCATION, (TOK), (EXP_FILENAME), \ > +> > > (EXP_LINENUM), (EXP_START_COL), (EXP_FINISH_COL)) > + > +/* Test of lexing a file using libcpp, verifying tokens and their > + location information. */ > + > +static void > +test_lexer (const line_table_case &case_) > +{ > + /* Create a tempfile and write some text to it. */ > + const char *content = > + /*00000000011111111112222222222333333.3333444444444.455555555556 > + 12345678901234567890123456789012345.6789012345678.901234567890. */ > + ("test_name /* c-style comment */\n" > + " \"test literal\"\n" > + " // test c++-style comment\n" > + " 42\n"); > + temp_source_file tmp (SELFTEST_LOCATION, ".txt", content); > + > + temp_line_table tmp_lt (case_); > + > + cpp_reader *parser = cpp_create_reader (CLK_GNUC89, NULL, line_table); > + > + const char *fname = cpp_read_main_file (parser, tmp.get_filename ()); > + ASSERT_NE (fname, NULL); > + > + /* Verify that we get the expected tokens back, with the correct > + location information. */ > + > + location_t loc; > + const cpp_token *tok; > + tok = cpp_get_token_with_location (parser, &loc); > + ASSERT_NE (tok, NULL); > + ASSERT_EQ (tok->type, CPP_NAME); > + ASSERT_TOKEN_AS_TEXT_EQ (parser, tok, "test_name"); > + ASSERT_TOKEN_LOC_EQ (tok, tmp.get_filename (), 1, 1, 9); > + > + tok = cpp_get_token_with_location (parser, &loc); > + ASSERT_NE (tok, NULL); > + ASSERT_EQ (tok->type, CPP_STRING); > + ASSERT_TOKEN_AS_TEXT_EQ (parser, tok, "\"test literal\""); > + ASSERT_TOKEN_LOC_EQ (tok, tmp.get_filename (), 2, 35, 48); > + > + tok = cpp_get_token_with_location (parser, &loc); > + ASSERT_NE (tok, NULL); > + ASSERT_EQ (tok->type, CPP_NUMBER); > + ASSERT_TOKEN_AS_TEXT_EQ (parser, tok, "42"); > + ASSERT_TOKEN_LOC_EQ (tok, tmp.get_filename (), 4, 4, 5); > + > + tok = cpp_get_token_with_location (parser, &loc); > + ASSERT_NE (tok, NULL); > + ASSERT_EQ (tok->type, CPP_EOF); > + > + cpp_finish (parser, NULL); > +} > + > +/* A table of interesting location_t values, giving one axis of our test > + matrix. */ > + > +static const location_t boundary_locations[] = { > + /* Zero means "don't override the default values for a new line_table". */ > + 0, > + > + /* An arbitrary non-zero value that isn't close to one of > + the boundary values below. */ > + 0x10000, > + > + /* Values near LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES. */ > + LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES - 0x100, > + LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES - 1, > + LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES, > + LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES + 1, > + 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 - 1, > + LINE_MAP_MAX_LOCATION_WITH_COLS, > + LINE_MAP_MAX_LOCATION_WITH_COLS + 1, > + LINE_MAP_MAX_LOCATION_WITH_COLS + 0x100, > +}; > + > /* Run all of the selftests within this file. */ > > void > input_c_tests () > { > - test_accessing_ordinary_linemaps (); > + test_should_have_column_data_p (); > test_unknown_location (); > test_builtins (); > + > + /* As noted above in the description of struct line_table_case, > + we want to explore a test matrix of interesting line_table > + situations, running various selftests for each case within the > + matrix. */ > + > + /* 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) > + { > + /* ...and use each of the "interesting" location values as > +> > the starting location within line_table. */ > + const int num_boundary_locations > +> > = sizeof (boundary_locations) / sizeof (boundary_locations[0]); > + for (int loc_idx = 0; loc_idx < num_boundary_locations; loc_idx++) > +> > { > +> > line_table_case c (default_range_bits, boundary_locations[loc_idx]); > + > +> > /* Run all tests for the given case within the test matrix. */ > +> > test_accessing_ordinary_linemaps (c); > +> > test_lexer (c); > + > +> > num_cases_tested++; > +> > } > + } > + > + /* Verify that we fully covered the test matrix. */ > + ASSERT_EQ (num_cases_tested, 2 * 12); > + > test_reading_source_line (); > } > > diff --git a/gcc/selftest.h b/gcc/selftest.h > index e719f5f..3906bed 100644 > --- a/gcc/selftest.h > +++ b/gcc/selftest.h > @@ -127,13 +127,19 @@ extern int num_passes; > ::selftest::pass if they are equal, > ::selftest::fail if they are non-equal. */ > > -#define ASSERT_EQ(EXPECTED, ACTUAL)> > > > \ > +#define ASSERT_EQ(EXPECTED, ACTUAL) \ > + ASSERT_EQ_AT ((SELFTEST_LOCATION), (EXPECTED), (ACTUAL)) > + > +/* Like ASSERT_EQ, but treat LOC as the effective location of the > + selftest. */ > + > +#define ASSERT_EQ_AT(LOC, EXPECTED, ACTUAL)> > > \ > SELFTEST_BEGIN_STMT> > > > > > \ > const char *desc = "ASSERT_EQ (" #EXPECTED ", " #ACTUAL ")"; \ > if ((EXPECTED) == (ACTUAL))> > > > > \ > - ::selftest::pass (SELFTEST_LOCATION, desc);> > > > > \ > + ::selftest::pass ((LOC), desc);> > > > \ > else> > > > > > > > \ > - ::selftest::fail (SELFTEST_LOCATION, desc);> > > > > \ > + ::selftest::fail ((LOC), desc);> > > > \ > SELFTEST_END_STMT > > /* Evaluate EXPECTED and ACTUAL and compare them with !=, calling > diff --git a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c > b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c > index 1c140d8..3644d9f 100644 > --- a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c > +++ b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c > @@ -87,11 +87,11 @@ plugin_init (struct plugin_name_args *plugin_info, > original_finalizer = diagnostic_finalizer (global_dc); > switch (base_location) > { > - case 0x50000001: > + case LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES + 1: > diagnostic_finalizer (global_dc) = verify_unpacked_ranges; > break; > > - case 0x60000001: > + case LINE_MAP_MAX_LOCATION_WITH_COLS + 1: > diagnostic_finalizer (global_dc) = verify_no_columns; > break; > > diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h > index 0f4b522..08c5aef 100644 > --- a/libcpp/include/line-map.h > +++ b/libcpp/include/line-map.h > @@ -260,6 +260,16 @@ typedef unsigned int linenum_type; > worked example in libcpp/location-example.txt. */ > typedef unsigned int source_location; > > +/* Do not pack ranges if locations get higher than this. > + If you change this, update: > + gcc.dg/plugin/location-overflow-test-*.c. */ > +const source_location LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES = 0x50000000; > + > +/* Do not track column numbers if locations get higher than this. > + If you change this, update: > + gcc.dg/plugin/location-overflow-test-*.c. */ > +const source_location LINE_MAP_MAX_LOCATION_WITH_COLS = 0x60000000; > + > /* A range of source locations. > > Ranges are closed: > diff --git a/libcpp/line-map.c b/libcpp/line-map.c > index 2e61895..d7c0d08 100644 > --- a/libcpp/line-map.c > +++ b/libcpp/line-map.c > @@ -31,18 +31,6 @@ along with this program; see the file COPYING3. If not see > disabled). */ > const unsigned int LINE_MAP_MAX_COLUMN_NUMBER = (1U << 12); > > -/* Do not pack ranges if locations get higher than this. > - If you change this, update: > - gcc.dg/plugin/location_overflow_plugin.c > - gcc.dg/plugin/location-overflow-test-*.c. */ > -const source_location LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES = 0x50000000; > - > -/* Do not track column numbers if locations get higher than this. > - If you change this, update: > - gcc.dg/plugin/location_overflow_plugin.c > - gcc.dg/plugin/location-overflow-test-*.c. */ > -const source_location LINE_MAP_MAX_LOCATION_WITH_COLS = 0x60000000; > - > /* Highest possible source location encoded within an ordinary or > macro map. */ > const source_location LINE_MAP_MAX_SOURCE_LOCATION = 0x70000000;
