Dear all,
Here is a patch that give us caret diagnostics in C/C++. There a lot
of things that can be improved but because I wanted to get some
feedback with my current approach.
Basically, I store a pointer linebuf in the line_map structure to a
character in the input file buffer. The character corresponds to the
first character in the line corresponding to TO_LINE in the line_map
structure. The downside of this is that the buffer cannot be freed
anymore. I am not sure whether this is better than storing a duplicate
of the line as gfortran does. The third approach would be to store an
offset and when generating diagnostics, reopen the file, fseek to the
offset and print that line.
One line_map can contain information about several lines, so we still
need to find the correct position for a line within linebuf. That is
what the hack in expand_location is for. It would be nice to have a
way to point directly to the beginning of each line: multiple pointers
per line_map?
Well, comments, ideas, code, questions, help are all welcome.
Cheers,
Manuel.
Index: gcc/tree.c
===================================================================
--- gcc/tree.c (revision 132680)
+++ gcc/tree.c (working copy)
@@ -3479,17 +3479,32 @@ expand_location (source_location loc)
if (loc == 0)
{
xloc.file = NULL;
xloc.line = 0;
xloc.column = 0;
+ xloc.linebuf = NULL;
}
else
{
const struct line_map *map = linemap_lookup (line_table, loc);
+ int i;
xloc.file = map->to_file;
xloc.line = SOURCE_LINE (map, loc);
xloc.column = SOURCE_COLUMN (map, loc);
+ xloc.linebuf = SOURCE_LINEBUFFER (map);
+ if (xloc.linebuf != NULL)
+ {
+ i = xloc.line - map->to_line;
+ for (;;)
+ {
+ if (i == 0 || *xloc.linebuf == '\0')
+ break;
+ if (*xloc.linebuf == '\n')
+ i--;
+ xloc.linebuf++;
+ }
+ }
};
return xloc;
}
Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c (revision 132680)
+++ gcc/diagnostic.c (working copy)
@@ -160,10 +160,47 @@ diagnostic_build_prefix (diagnostic_info
: flag_show_column && s.column != 0
? build_message_string ("%s:%d:%d: %s", s.file, s.line, s.column, text)
: build_message_string ("%s:%d: %s", s.file, s.line, text));
}
+
+#define DIAGNOSTICS_CARET_CHAR '^'
+static const unsigned char *
+get_source_line (expanded_location s)
+{
+ return s.linebuf;
+}
+static void
+diagnostic_show_locus (diagnostic_context * context ATTRIBUTE_UNUSED,
+ diagnostic_info *diagnostic)
+{
+ const unsigned char *line;
+ int max_width = 80;
+ expanded_location s;
+
+ if (!context->show_caret)
+ return;
+
+ s = expand_location (diagnostic->location);
+ line = get_source_line (s);
+ if (line == NULL)
+ return;
+
+ putchar (' ');
+
+ while (max_width > 0 && *line != '\0' && *line != '\n')
+ {
+ max_width--;
+ putchar (*line);
+ line++;
+ }
+ putchar('\n');
+ gcc_assert (s.column > 0);
+ printf (" %*c\n", s.column, DIAGNOSTICS_CARET_CHAR);
+}
+
+
/* Count a diagnostic. Return true if the message should be printed. */
static bool
diagnostic_count_diagnostic (diagnostic_context *context,
diagnostic_info *diagnostic)
{
@@ -418,10 +455,11 @@ diagnostic_report_diagnostic (diagnostic
pp_format (context->printer, &diagnostic->message);
(*diagnostic_starter (context)) (context, diagnostic);
pp_output_formatted_text (context->printer);
(*diagnostic_finalizer (context)) (context, diagnostic);
pp_flush (context->printer);
+ diagnostic_show_locus (context, diagnostic);
diagnostic_action_after_output (context, diagnostic);
diagnostic->message.format_spec = saved_format_spec;
diagnostic->abstract_origin = NULL;
}
Index: gcc/diagnostic.h
===================================================================
--- gcc/diagnostic.h (revision 132680)
+++ gcc/diagnostic.h (working copy)
@@ -85,10 +85,14 @@ struct diagnostic_context
/* True if we should print the command line option which controls
each diagnostic, if known. */
bool show_option_requested;
+ /* True if we should print the source line with a caret indicating
+ the location. */
+ bool show_caret;
+
/* True if we should raise a SIGABRT on errors. */
bool abort_on_error;
/* This function is called before any message is printed out. It is
responsible for preparing message prefix and such. For example, it
Index: gcc/input.h
===================================================================
--- gcc/input.h (revision 132680)
+++ gcc/input.h (working copy)
@@ -39,10 +39,13 @@ typedef struct GTY (())
/* The line-location in the source file. */
int line;
int column;
+
+ /* The offset location in the source file. */
+ const unsigned char *linebuf;
} expanded_location;
extern expanded_location expand_location (source_location);
/* Historically GCC used location_t, while cpp used source_location.
Index: gcc/testsuite/gcc.dg/caret-1.c
===================================================================
--- gcc/testsuite/gcc.dg/caret-1.c (revision 0)
+++ gcc/testsuite/gcc.dg/caret-1.c (revision 0)
@@ -0,0 +1,9 @@
+void foo ()
+{
+ label1:
+ foo ();
+ label1: /* repeated */
+ foo ();
+}
+
+
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c (revision 132680)
+++ gcc/cp/decl.c (working copy)
@@ -11801,11 +11801,11 @@ finish_function (int flags)
#endif
/* Hack. We don't want the middle-end to warn that this
return is unreachable, so put the statement on the
special line 0. */
{
- location_t linezero = linemap_line_start (line_table, 0, 1);
+ location_t linezero = linemap_line_start (line_table, 0, 1, NULL);
SET_EXPR_LOCATION (stmt, linezero);
}
}
if (use_eh_spec_block (current_function_decl))
Index: gcc/c-pch.c
===================================================================
--- gcc/c-pch.c (revision 132680)
+++ gcc/c-pch.c (working copy)
@@ -420,11 +420,11 @@ c_common_read_pch (cpp_reader *pfile, co
return;
fclose (f);
cpp_set_line_map (pfile, line_table);
- linemap_add (line_table, LC_RENAME, 0, saved_loc.file, saved_loc.line);
+ linemap_add (line_table, LC_RENAME, 0, saved_loc.file, saved_loc.line, NULL);
/* Give the front end a chance to take action after a PCH file has
been loaded. */
if (lang_post_pch_load)
(*lang_post_pch_load) ();
Index: gcc/opts.c
===================================================================
--- gcc/opts.c (revision 132680)
+++ gcc/opts.c (working copy)
@@ -1655,10 +1655,14 @@ common_handle_option (size_t scode, cons
case OPT_fdiagnostics_show_option:
global_dc->show_option_requested = true;
break;
+ case OPT_fdiagnostics_show_caret:
+ global_dc->show_caret = true;
+ break;
+
case OPT_fdump_:
if (!dump_switch_p (arg))
return 0;
break;
Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c (revision 132680)
+++ gcc/c-opts.c (working copy)
@@ -1501,11 +1501,11 @@ finish_options (void)
{
size_t i;
cb_file_change (parse_in,
linemap_add (line_table, LC_RENAME, 0,
- _("<built-in>"), 0));
+ _("<built-in>"), 0, NULL));
cpp_init_builtins (parse_in, flag_hosted);
c_cpp_builtins (parse_in);
/* We're about to send user input to cpplib, so make it warn for
@@ -1519,11 +1519,11 @@ finish_options (void)
their acceptance on the -std= setting. */
cpp_opts->warn_dollars = (cpp_opts->pedantic && !cpp_opts->c99);
cb_file_change (parse_in,
linemap_add (line_table, LC_RENAME, 0,
- _("<command-line>"), 0));
+ _("<command-line>"), 0, NULL));
for (i = 0; i < deferred_count; i++)
{
struct deferred_opt *opt = &deferred_opts[i];
Index: gcc/common.opt
===================================================================
--- gcc/common.opt (revision 132680)
+++ gcc/common.opt (working copy)
@@ -444,10 +444,14 @@ Common Joined RejectNegative
fdiagnostics-show-option
Common
Amend appropriate diagnostic messages with the command line option that
controls them
+fdiagnostics-show-caret
+Common
+Show the source line with a caret indicating the column
+
fdump-
Common Joined RejectNegative
-fdump-<type> Dump various compiler internals to a file
fdump-noaddr
Index: libcpp/directives.c
===================================================================
--- libcpp/directives.c (revision 132680)
+++ libcpp/directives.c (working copy)
@@ -982,14 +982,17 @@ do_linemarker (cpp_reader *pfile)
void
_cpp_do_file_change (cpp_reader *pfile, enum lc_reason reason,
const char *to_file, unsigned int file_line,
unsigned int sysp)
{
- const struct line_map *map = linemap_add (pfile->line_table, reason, sysp,
- to_file, file_line);
+ const struct line_map *map;
+ const unsigned char *linebuf = (pfile->buffer == NULL) ? NULL :
pfile->buffer->next_line;
+
+ map = linemap_add (pfile->line_table, reason, sysp,
+ to_file, file_line, linebuf);
if (map != NULL)
- linemap_line_start (pfile->line_table, map->to_line, 127);
+ linemap_line_start (pfile->line_table, map->to_line, 127, linebuf);
if (pfile->cb.file_change)
pfile->cb.file_change (pfile, map);
}
Index: libcpp/include/line-map.h
===================================================================
--- libcpp/include/line-map.h (revision 132680)
+++ libcpp/include/line-map.h (working copy)
@@ -63,10 +63,12 @@ struct line_map GTY(())
ENUM_BITFIELD (lc_reason) reason : CHAR_BIT;
/* The sysp field isn't really needed now that it's in cpp_buffer. */
unsigned char sysp;
/* Number of the low-order source_location bits used for a column number. */
unsigned int column_bits : 8;
+ /* The input line. */
+ const unsigned char * linebuf;
};
/* A set of chronological line_map structures. */
struct line_maps GTY(())
{
@@ -117,11 +119,11 @@ extern void linemap_check_files_exited (
most recent linemap_add). MAX_COLUMN_HINT is the highest column
number we expect to use in this line (but it does not change
the highest_location). */
extern source_location linemap_line_start
-(struct line_maps *set, unsigned int to_line, unsigned int max_column_hint);
+(struct line_maps *set, unsigned int to_line, unsigned int max_column_hint,
const unsigned char *linebuf);
/* Add a mapping of logical source line to physical source file and
line number.
The text pointed to by TO_FILE must have a lifetime
@@ -132,11 +134,11 @@ extern source_location linemap_line_star
A call to this function can relocate the previous set of
maps, so any stored line_map pointers should not be used. */
extern const struct line_map *linemap_add
(struct line_maps *, enum lc_reason, unsigned int sysp,
- const char *to_file, unsigned int to_line);
+ const char *to_file, unsigned int to_line, const unsigned char *linebuf);
/* Given a logical line, returns the map from which the corresponding
(source file, line) pair can be deduced. */
extern const struct line_map *linemap_lookup
(struct line_maps *, source_location);
@@ -146,15 +148,18 @@ extern const struct line_map *linemap_lo
the most recently listed stack is the same as the current one. */
extern void linemap_print_containing_files (struct line_maps *,
const struct line_map *);
/* Converts a map and a source_location to source line. */
-#define SOURCE_LINE(MAP, LINE) \
- ((((LINE) - (MAP)->start_location) >> (MAP)->column_bits) + (MAP)->to_line)
+#define SOURCE_LINE(MAP, LOCATION) \
+ ((((LOCATION) - (MAP)->start_location) >> (MAP)->column_bits) +
(MAP)->to_line)
-#define SOURCE_COLUMN(MAP, LINE) \
- (((LINE) - (MAP)->start_location) & ((1 << (MAP)->column_bits) - 1))
+#define SOURCE_COLUMN(MAP, LOCATION) \
+ (((LOCATION) - (MAP)->start_location) & ((1 << (MAP)->column_bits) - 1))
+
+#define SOURCE_LINEBUFFER(MAP) \
+ ((MAP)->linebuf)
/* Returns the last source line within a map. This is the (last) line
of the #include, or other directive, that caused a map change. */
#define LAST_SOURCE_LINE(MAP) \
SOURCE_LINE (MAP, LAST_SOURCE_LINE_LOCATION (MAP))
Index: libcpp/files.c
===================================================================
--- libcpp/files.c (revision 132680)
+++ libcpp/files.c (working copy)
@@ -1306,11 +1306,11 @@ _cpp_pop_file_buffer (cpp_reader *pfile,
/* Invalidate control macros in the #including file. */
pfile->mi_valid = false;
if (file->buffer)
{
- free ((void *) file->buffer);
+ /* free ((void *) file->buffer);*/
file->buffer = NULL;
file->buffer_valid = false;
}
}
Index: libcpp/line-map.c
===================================================================
--- libcpp/line-map.c (revision 132680)
+++ libcpp/line-map.c (working copy)
@@ -83,11 +83,11 @@ linemap_free (struct line_maps *set)
function. A call to this function can relocate the previous set of
maps, so any stored line_map pointers should not be used. */
const struct line_map *
linemap_add (struct line_maps *set, enum lc_reason reason,
- unsigned int sysp, const char *to_file, unsigned int to_line)
+ unsigned int sysp, const char *to_file, unsigned int to_line,
const unsigned char *linebuf)
{
struct line_map *map;
source_location start_location = set->highest_location + 1;
if (set->used && start_location < set->maps[set->used - 1].start_location)
@@ -155,16 +155,19 @@ linemap_add (struct line_maps *set, enum
map->reason = reason;
map->sysp = sysp;
map->start_location = start_location;
map->to_file = to_file;
map->to_line = to_line;
+ if (linebuf != NULL)
+ map->linebuf = linebuf;
set->cache = set->used++;
map->column_bits = 0;
set->highest_location = start_location;
set->highest_line = start_location;
set->max_column_hint = 0;
+
if (reason == LC_ENTER)
{
map->included_from = set->depth == 0 ? -1 : (int) (set->used - 2);
set->depth++;
if (set->trace_includes)
@@ -181,11 +184,11 @@ linemap_add (struct line_maps *set, enum
return map;
}
source_location
linemap_line_start (struct line_maps *set, unsigned int to_line,
- unsigned int max_column_hint)
+ unsigned int max_column_hint, const unsigned char *linebuf)
{
struct line_map *map = &set->maps[set->used - 1];
source_location highest = set->highest_location;
source_location r;
unsigned int last_line = SOURCE_LINE (map, set->highest_line);
@@ -198,10 +201,11 @@ linemap_line_start (struct line_maps *se
{
add_map = true;
}
else
max_column_hint = set->max_column_hint;
+
if (add_map)
{
int column_bits;
if (max_column_hint > 100000 || highest > 0xC0000000)
{
@@ -223,21 +227,24 @@ linemap_line_start (struct line_maps *se
single line we can sometimes just increase its column_bits instead. */
if (line_delta < 0
|| last_line != map->to_line
|| SOURCE_COLUMN (map, highest) >= (1U << column_bits))
map = (struct line_map *) linemap_add (set, LC_RENAME, map->sysp,
- map->to_file, to_line);
+ map->to_file, to_line, linebuf);
map->column_bits = column_bits;
r = map->start_location + ((to_line - map->to_line) << column_bits);
}
else
r = highest - SOURCE_COLUMN (map, highest)
+ (line_delta << map->column_bits);
set->highest_line = r;
if (r > set->highest_location)
set->highest_location = r;
set->max_column_hint = max_column_hint;
+ /* if (linebuf != NULL)
+ map->linebuf = linebuf;*/
+ linebuf = 0;
return r;
}
source_location
linemap_position_for_column (struct line_maps *set, unsigned int to_column)
@@ -251,11 +258,12 @@ linemap_position_for_column (struct line
return r;
}
else
{
struct line_map *map = &set->maps[set->used - 1];
- r = linemap_line_start (set, SOURCE_LINE (map, r), to_column + 50);
+ r = linemap_line_start (set, SOURCE_LINE (map, r), to_column + 50,
+ SOURCE_LINEBUFFER (map));
}
}
r = r + to_column;
if (r >= set->highest_location)
set->highest_location = r;
Index: libcpp/internal.h
===================================================================
--- libcpp/internal.h (revision 132680)
+++ libcpp/internal.h (working copy)
@@ -66,11 +66,11 @@ struct cset_converter
#define CPP_INCREMENT_LINE(PFILE, COLS_HINT) do { \
const struct line_maps *line_table = PFILE->line_table; \
const struct line_map *map = &line_table->maps[line_table->used-1]; \
unsigned int line = SOURCE_LINE (map, line_table->highest_line); \
- linemap_line_start (PFILE->line_table, line + 1, COLS_HINT); \
+ linemap_line_start (PFILE->line_table, line + 1, COLS_HINT,
PFILE->buffer->cur); \
} while (0)
/* Maximum nesting of cpp_buffers. We use a static limit, partly for
efficiency, and partly to limit runaway recursion. */
#define CPP_STACK_MAX 200
