On Wed, 2023-08-09 at 18:14 -0400, Lewis Hyatt wrote:
> The previous patch in this series introduced the concept of LC_GEN line
> maps. This patch continues on the path to using them to improve _Pragma
> diagnostics, by adding a new source_id SRC member to struct
> expanded_location, which is populated by linemap_expand_location. This
> member allows call sites to detect and handle when a location refers to
> generated data rather than a plain file name.
>
> The previous FILE member of expanded_location is preserved (although
> redundant with SRC), so that call sites which do not and never will care
> about generated data do not need to be concerned about it. Call sites that
> will care are modified here, to use SRC rather than FILE for comparing
> locations.
Thanks; this seems like a good approach.
[...snip...]
> diff --git a/gcc/edit-context.cc b/gcc/edit-context.cc
> index 6f5bc6b9d8f..15052aec417 100644
> --- a/gcc/edit-context.cc
> +++ b/gcc/edit-context.cc
> @@ -295,7 +295,7 @@ edit_context::apply_fixit (const fixit_hint *hint)
> {
> expanded_location start = expand_location (hint->get_start_loc ());
> expanded_location next_loc = expand_location (hint->get_next_loc ());
> - if (start.file != next_loc.file)
> + if (start.src != next_loc.src || start.src.is_buffer ())
> return false;
> if (start.line != next_loc.line)
> return false;
Thinking about fix-it hints, it makes sense to reject attempts to
create fix-it hints within generated strings, as we can't apply them or
visualize them.
Does anywhere in the patch kit do that? Either of
rich_location::maybe_add_fixit
or
rich_location::reject_impossible_fixit
would be good places to do that.
[...snip...]
> diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
> index e59123b18c5..76617fe6129 100644
> --- a/libcpp/include/line-map.h
> +++ b/libcpp/include/line-map.h
> @@ -1410,18 +1410,22 @@ linemap_location_before_p (class line_maps *set,
>
> typedef struct
> {
> - /* The name of the source file involved. */
> - const char *file;
> + /* The file name of the location involved, or NULL if the location
> + is not in an external file. */
> + const char *file = nullptr;
>
> - /* The line-location in the source file. */
> - int line;
> -
> - int column;
> + /* A source_id recording the file name and/or the in-memory content,
> + as appropriate. Users that need to handle in-memory content need
> + to use this rather than FILE. */
> + source_id src;
>
> - void *data;
> + /* The line-location in the source file. */
> + int line = 0;
> + int column = 0;
> + void *data = nullptr;
>
> - /* In a system header?. */
> - bool sysp;
> + /* In a system header? */
> + bool sysp = false;
> } expanded_location;
I don't know if we've been using default member initialization yet, but
apparently it's C++11, and thus OK.
[...snip...]
This patch looks good to me, but obviously it has dependencies on the
rest of the kit.
Dave
