From b0d11f1ed68727a85280e0ceb83bf77b5385902a Mon Sep 17 00:00:00 2001 From: Nathan Sidwell Date: Thu, 29 Aug 2019 14:06:32 +0000 Subject: [PATCH] [preprocessor] Include stacking https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01971.html * internal.h (enum include_type): Add IT_MAIN, IT_DIRECTIVE_HWM, IT_HEADER_HWM. (_cpp_stack_file): Take include_type, not a bool. * files.c (_cpp_find_file): Refactor to not hide an if inside a for conditional. (should_stack_file): Break apart to ... (is_known_idempotent_file, has_unique_contents): ... these. (_cpp_stack_file): Replace IMPORT boolean with include_type enum. Refactor to use new predicates. Do linemap compensation here ... (_cpp_stack_include): ... not here. * init.c (cpp_read_main_file): Pass IT_MAIN to _cpp_stack_file. From-SVN: r275034 --- libcpp/ChangeLog | 14 +++ libcpp/files.c | 297 ++++++++++++++++++++++------------------------ libcpp/init.c | 2 +- libcpp/internal.h | 7 +- 4 files changed, 165 insertions(+), 155 deletions(-) diff --git a/libcpp/ChangeLog b/libcpp/ChangeLog index 61d91acb2d7..8af0ad2eacb 100644 --- a/libcpp/ChangeLog +++ b/libcpp/ChangeLog @@ -1,3 +1,17 @@ +2019-08-29 Nathan Sidwell + + * internal.h (enum include_type): Add IT_MAIN, IT_DIRECTIVE_HWM, + IT_HEADER_HWM. + (_cpp_stack_file): Take include_type, not a bool. + * files.c (_cpp_find_file): Refactor to not hide an if inside a + for conditional. + (should_stack_file): Break apart to ... + (is_known_idempotent_file, has_unique_contents): ... these. + (_cpp_stack_file): Replace IMPORT boolean with include_type enum. + Refactor to use new predicates. Do linemap compensation here ... + (_cpp_stack_include): ... not here. + * init.c (cpp_read_main_file): Pass IT_MAIN to _cpp_stack_file. + 2019-08-28 Nathan Sidwell * directives-only.c (_cpp_preprocess_dir_only): Use false, not diff --git a/libcpp/files.c b/libcpp/files.c index 30c0cf25519..ee31aff606d 100644 --- a/libcpp/files.c +++ b/libcpp/files.c @@ -176,8 +176,6 @@ static bool read_file_guts (cpp_reader *pfile, _cpp_file *file, location_t loc); static bool read_file (cpp_reader *pfile, _cpp_file *file, location_t loc); -static bool should_stack_file (cpp_reader *, _cpp_file *file, bool import, - location_t loc); static struct cpp_dir *search_path_head (cpp_reader *, const char *fname, int angle_brackets, enum include_type); static const char *dir_name_of_file (_cpp_file *file); @@ -536,79 +534,86 @@ _cpp_find_file (cpp_reader *pfile, const char *fname, cpp_dir *start_dir, || (pfile->buffer && pfile->buffer->file->implicit_preinclude)); - /* Try each path in the include chain. */ - for (; !fake ;) - { - if (find_file_in_dir (pfile, file, &invalid_pch, loc)) - break; - - file->dir = file->dir->next; - if (file->dir == NULL) - { - if (search_path_exhausted (pfile, fname, file)) - { - /* Although this file must not go in the cache, because - the file found might depend on things (like the current file) - that aren't represented in the cache, it still has to go in - the list of all files so that #import works. */ - file->next_file = pfile->all_files; - pfile->all_files = file; - if (*hash_slot == NULL) - { - /* If *hash_slot is NULL, the above htab_find_slot_with_hash - call just created the slot, but we aren't going to store - there anything, so need to remove the newly created entry. - htab_clear_slot requires that it is non-NULL, so store - there some non-NULL pointer, htab_clear_slot will - overwrite it immediately. */ - *hash_slot = file; - htab_clear_slot (pfile->file_hash, hash_slot); - } - return file; - } - - if (invalid_pch) - { - cpp_error (pfile, CPP_DL_ERROR, - "one or more PCH files were found, but they were invalid"); - if (!cpp_get_options (pfile)->warn_invalid_pch) - cpp_error (pfile, CPP_DL_ERROR, - "use -Winvalid-pch for more information"); - } - if (implicit_preinclude) - { - free ((char *) file->name); - free (file); - if (*hash_slot == NULL) - { - /* See comment on the above htab_clear_slot call. */ - *hash_slot = file; - htab_clear_slot (pfile->file_hash, hash_slot); - } - return NULL; - } - else - open_file_failed (pfile, file, angle_brackets, loc); + if (!fake) + /* Try each path in the include chain. */ + for (;;) + { + if (find_file_in_dir (pfile, file, &invalid_pch, loc)) break; - } - /* Only check the cache for the starting location (done above) - and the quote and bracket chain heads because there are no - other possible starting points for searches. */ - if (file->dir == pfile->bracket_include) - saw_bracket_include = true; - else if (file->dir == pfile->quote_include) - saw_quote_include = true; - else - continue; + file->dir = file->dir->next; + if (file->dir == NULL) + { + if (search_path_exhausted (pfile, fname, file)) + { + /* Although this file must not go in the cache, + because the file found might depend on things (like + the current file) that aren't represented in the + cache, it still has to go in the list of all files + so that #import works. */ + file->next_file = pfile->all_files; + pfile->all_files = file; + if (*hash_slot == NULL) + { + /* If *hash_slot is NULL, the above + htab_find_slot_with_hash call just created the + slot, but we aren't going to store there + anything, so need to remove the newly created + entry. htab_clear_slot requires that it is + non-NULL, so store there some non-NULL pointer, + htab_clear_slot will overwrite it + immediately. */ + *hash_slot = file; + htab_clear_slot (pfile->file_hash, hash_slot); + } + return file; + } + + if (invalid_pch) + { + cpp_error (pfile, CPP_DL_ERROR, + "one or more PCH files were found," + " but they were invalid"); + if (!cpp_get_options (pfile)->warn_invalid_pch) + cpp_error (pfile, CPP_DL_ERROR, + "use -Winvalid-pch for more information"); + } + + if (implicit_preinclude) + { + free ((char *) file->name); + free (file); + if (*hash_slot == NULL) + { + /* See comment on the above htab_clear_slot call. */ + *hash_slot = file; + htab_clear_slot (pfile->file_hash, hash_slot); + } + return NULL; + } - entry = search_cache ((struct cpp_file_hash_entry *) *hash_slot, file->dir); - if (entry) - { - found_in_cache = file->dir; - break; - } - } + open_file_failed (pfile, file, angle_brackets, loc); + break; + } + + /* Only check the cache for the starting location (done above) + and the quote and bracket chain heads because there are no + other possible starting points for searches. */ + if (file->dir == pfile->bracket_include) + saw_bracket_include = true; + else if (file->dir == pfile->quote_include) + saw_quote_include = true; + else + continue; + + entry + = search_cache ((struct cpp_file_hash_entry *) *hash_slot, file->dir); + if (entry) + { + found_in_cache = file->dir; + break; + } + } if (entry) { @@ -778,18 +783,14 @@ read_file (cpp_reader *pfile, _cpp_file *file, location_t loc) return !file->dont_read; } -/* Returns TRUE if FILE's contents have been successfully placed in - FILE->buffer and the file should be stacked, otherwise false. - Use LOC for any diagnostics. */ +/* Returns TRUE if FILE is already known to be idempotent, and should + therefore not be read again. */ static bool -should_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, - location_t loc) +is_known_idempotent_file (cpp_reader *pfile, _cpp_file *file, bool import) { - _cpp_file *f; - /* Skip once-only files. */ if (file->once_only) - return false; + return true; /* We must mark the file once-only if #import now, before header guard checks. Otherwise, undefining the header guard might @@ -800,13 +801,13 @@ should_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, /* Don't stack files that have been stacked before. */ if (file->stack_count) - return false; + return true; } /* Skip if the file had a header guard and the macro is defined. PCH relies on this appearing before the PCH handler below. */ if (file->cmacro && cpp_macro_p (file->cmacro)) - return false; + return true; /* Handle PCH files immediately; don't stack them. */ if (file->pchname) @@ -815,12 +816,19 @@ should_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, file->fd = -1; free ((void *) file->pchname); file->pchname = NULL; - return false; + return true; } - if (!read_file (pfile, file, loc)) - return false; + return false; +} +/* Return TRUE if file has unique contents, so we should read process + it. The file's contents must already have been read. */ + +static bool +has_unique_contents (cpp_reader *pfile, _cpp_file *file, bool import, + location_t loc) +{ /* Check the file against the PCH file. This is done before checking against files we've already seen, since it may save on I/O. */ @@ -841,10 +849,10 @@ should_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, /* We may have read the file under a different name. Look for likely candidates and compare file contents to be sure. */ - for (f = pfile->all_files; f; f = f->next_file) + for (_cpp_file *f = pfile->all_files; f; f = f->next_file) { if (f == file) - continue; + continue; /* It'sa me! */ if ((import || f->once_only) && f->err_no == 0 @@ -852,7 +860,6 @@ should_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, && f->st.st_size == file->st.st_size) { _cpp_file *ref_file; - bool same_file_p = false; if (f->buffer && !f->buffer_valid) { @@ -865,12 +872,11 @@ should_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, /* The file is not stacked anymore. We can reuse it. */ ref_file = f; - same_file_p = read_file (pfile, ref_file, loc) - /* Size might have changed in read_file(). */ - && ref_file->st.st_size == file->st.st_size - && !memcmp (ref_file->buffer, - file->buffer, - file->st.st_size); + bool same_file_p = (read_file (pfile, ref_file, loc) + /* Size might have changed in read_file(). */ + && ref_file->st.st_size == file->st.st_size + && !memcmp (ref_file->buffer, file->buffer, + file->st.st_size)); if (f->buffer && !f->buffer_valid) { @@ -879,11 +885,12 @@ should_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, } if (same_file_p) - break; + /* Already seen under a different name. */ + return false; } } - return f == NULL; + return true; } /* Place the file referenced by FILE into a new buffer on the buffer @@ -891,25 +898,27 @@ should_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, because of a #import directive. Returns true if a buffer is stacked. Use LOC for any diagnostics. */ bool -_cpp_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, +_cpp_stack_file (cpp_reader *pfile, _cpp_file *file, include_type type, location_t loc) { - cpp_buffer *buffer; - int sysp; + if (is_known_idempotent_file (pfile, file, type == IT_IMPORT)) + return false; - if (!should_stack_file (pfile, file, import, loc)) + if (!read_file (pfile, file, loc)) return false; - if (pfile->buffer == NULL || file->dir == NULL) - sysp = 0; - else - sysp = MAX (pfile->buffer->sysp, file->dir->sysp); + if (!has_unique_contents (pfile, file, type == IT_IMPORT, loc)) + return false; + + int sysp = 0; + if (pfile->buffer && file->dir) + sysp = MAX (pfile->buffer->sysp, file->dir->sysp); /* Add the file to the dependencies on its first inclusion. */ - if (!file->stack_count - && CPP_OPTION (pfile, deps.style) > !!sysp + if (CPP_OPTION (pfile, deps.style) > (sysp != 0) + && !file->stack_count && file->path[0] - && (!file->main_file || !CPP_OPTION (pfile, deps.ignore_main_file))) + && !(file->main_file && CPP_OPTION (pfile, deps.ignore_main_file))) deps_add_dep (pfile->deps, file->path); /* Clear buffer_valid since _cpp_clean_line messes it up. */ @@ -917,9 +926,10 @@ _cpp_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, file->stack_count++; /* Stack the buffer. */ - buffer = cpp_push_buffer (pfile, file->buffer, file->st.st_size, - CPP_OPTION (pfile, preprocessed) - && !CPP_OPTION (pfile, directives_only)); + cpp_buffer *buffer + = cpp_push_buffer (pfile, file->buffer, file->st.st_size, + CPP_OPTION (pfile, preprocessed) + && !CPP_OPTION (pfile, directives_only)); buffer->file = file; buffer->sysp = sysp; buffer->to_free = file->buffer_start; @@ -928,7 +938,27 @@ _cpp_stack_file (cpp_reader *pfile, _cpp_file *file, bool import, pfile->mi_valid = true; pfile->mi_cmacro = 0; - /* Generate the call back. */ + /* Compensate for the increment in linemap_add that occurs when in + do_file_change. In the case of a normal #include, we're + currently at the start of the line *following* the #include. A + separate location_t for this location makes no sense (until we do + the LC_LEAVE), and complicates LAST_SOURCE_LINE_LOCATION. This + does not apply if we found a PCH file (in which case linemap_add + is not called) or we were included from the command-line. In the + case that the #include is the last line in the file, + highest_location still points to the current line, not the start + of the next line, so we do not decrement in this case. See + plugin/location-overflow-test-pr83173.h for an example. */ + bool decremented = false; + if (file->pchname == NULL && file->err_no == 0 && type < IT_DIRECTIVE_HWM) + { + decremented = (pfile->line_table->highest_line + == pfile->line_table->highest_location); + if (decremented) + pfile->line_table->highest_location--; + } + + /* Add line map and do callbacks. */ _cpp_do_file_change (pfile, LC_ENTER, file->path, 1, sysp); return true; @@ -1009,11 +1039,6 @@ bool _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, enum include_type type, location_t loc) { - struct cpp_dir *dir; - _cpp_file *file; - bool stacked; - bool decremented = false; - /* For -include command-line flags we have type == IT_CMDLINE. When the first -include file is processed we have the case, where pfile->cur_token == pfile->cur_run->base, we are directly called up @@ -1026,48 +1051,16 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, if (type == IT_CMDLINE && pfile->cur_token != pfile->cur_run->base) pfile->cur_token[-1].src_loc = 0; - dir = search_path_head (pfile, fname, angle_brackets, type); + cpp_dir *dir = search_path_head (pfile, fname, angle_brackets, type); if (!dir) return false; - file = _cpp_find_file (pfile, fname, dir, false, angle_brackets, - type == IT_DEFAULT, loc); + _cpp_file *file = _cpp_find_file (pfile, fname, dir, false, angle_brackets, + type == IT_DEFAULT, loc); if (type == IT_DEFAULT && file == NULL) return false; - /* Compensate for the increment in linemap_add that occurs if - _cpp_stack_file actually stacks the file. In the case of a normal - #include, we're currently at the start of the line *following* the - #include. A separate location_t for this location makes no - sense (until we do the LC_LEAVE), and complicates - LAST_SOURCE_LINE_LOCATION. This does not apply if we found a PCH - file (in which case linemap_add is not called) or we were included - from the command-line. In the case that the #include is the last - line in the file, highest_location still points to the current - line, not the start of the next line, so we do not decrement in - this case. See plugin/location-overflow-test-pr83173.h for an - example. */ - if (file->pchname == NULL && file->err_no == 0 - && type != IT_CMDLINE && type != IT_DEFAULT) - { - int highest_line = linemap_get_expansion_line (pfile->line_table, - pfile->line_table->highest_location); - int source_line = linemap_get_expansion_line (pfile->line_table, loc); - if (highest_line > source_line) - { - pfile->line_table->highest_location--; - decremented = true; - } - } - - stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); - - if (decremented && !stacked) - /* _cpp_stack_file didn't stack the file, so let's rollback the - compensation dance we performed above. */ - pfile->line_table->highest_location++; - - return stacked; + return _cpp_stack_file (pfile, file, type, loc); } /* Could not open FILE. The complication is dependency output. */ diff --git a/libcpp/init.c b/libcpp/init.c index 472f104cb5f..ccbfc96489d 100644 --- a/libcpp/init.c +++ b/libcpp/init.c @@ -651,7 +651,7 @@ cpp_read_main_file (cpp_reader *pfile, const char *fname) if (_cpp_find_failed (pfile->main_file)) return NULL; - _cpp_stack_file (pfile, pfile->main_file, false, loc); + _cpp_stack_file (pfile, pfile->main_file, IT_MAIN, 0); /* For foo.i, read the original filename foo.c now, for the benefit of the front ends. */ diff --git a/libcpp/internal.h b/libcpp/internal.h index a86b58785e9..d4768aee713 100644 --- a/libcpp/internal.h +++ b/libcpp/internal.h @@ -123,6 +123,10 @@ enum include_type /* Non-directive including mechanisms. */ IT_CMDLINE, /* -include */ IT_DEFAULT, /* forced header */ + IT_MAIN, /* main */ + + IT_DIRECTIVE_HWM = IT_IMPORT + 1, /* Directives below this. */ + IT_HEADER_HWM = IT_DEFAULT + 1, /* Header files below this. */ }; union utoken @@ -671,8 +675,7 @@ extern _cpp_file *_cpp_find_file (cpp_reader *, const char *, cpp_dir *, extern bool _cpp_find_failed (_cpp_file *); extern void _cpp_mark_file_once_only (cpp_reader *, struct _cpp_file *); extern void _cpp_fake_include (cpp_reader *, const char *); -extern bool _cpp_stack_file (cpp_reader *, _cpp_file*, bool, - location_t); +extern bool _cpp_stack_file (cpp_reader *, _cpp_file*, include_type, location_t); extern bool _cpp_stack_include (cpp_reader *, const char *, int, enum include_type, location_t); extern int _cpp_compare_file_date (cpp_reader *, const char *, int); -- 2.30.2