Re: [PATCH] caret diagnostics

[Tom Tromey <tromey@redhat.com>]

>>>>> "Manuel" == Manuel López-Ibáñez <lopezibanez@gmail.com> writes:

Manuel> The approach followed is to never free the input buffers and keep
Manuel> pointers to the start of each line tracked by each line-map.

This seems reasonable, definitely so for a first patch.

Manuel> b) Re-open the file and fseek but that is not trivial since we need to
Manuel> do it fast but still do all character conversions that we did when
Manuel> libcpp opened it the first time.

One argument in favor of this approach is that in a given compilation
we don't emit diagnostics for most source files.  So, why pay the
price of keeping those files in memory?

There are various tradeoffs we could play with.  Like, if we commonly
do not apply character set conversions, we could just mmap the files
and leave them open.  There's a lot of details here I don't remember
offhand, though (like, what kinds of weird stuff does libcpp do with
buffers?).

Manuel> Index: gcc/java/jcf-parse.c
[...]
Manuel> +      /* FIXME CARET: We should add a pointer to the input line
Manuel> +	 instead of NULL.  */

FWIW for gcj, carets don't really make sense, since we are only
compiling class files.  We rely on ecj to emit the nice errors :)

Manuel> +#ifdef USE_CARET_DIAGNOSTICS
Manuel> +      xloc.linebuf = NULL;
Manuel> +#endif

I'm with Joseph on this one -- just add the code, don't make it
conditional.  Mapped locations are not a good historical model to
follow.

Manuel> +  while (max_width > 0 && *line != '\0' && *line != '\n')
Manuel> +    {
Manuel> +      max_width--;
Manuel> +      putchar (*line);
Manuel> +      line++;
Manuel> +    }
Manuel> +  putchar('\n');
Manuel> +  gcc_assert (s.column > 0);
Manuel> +  printf (" %*c\n", s.column, '^');

I think this will run into two problems.

First, the libcpp buffer will be in UTF-8 (or UTF-EBCDIC -- nice), but
the user might be using a different charset.  So, I think we have to
iconv here.

Also, my recollection is that libcpp encodes the column by counting
characters, not by counting columns-as-would-be-displayed.  So, this
code has to have some special handling for TAB characters.

Manuel> Index: gcc/c-tree.h

This seems to be an unrelated change.

I'm not super fond of the inline function SOURCE_LINEBUFFER.  All
those caps read strangely.  And, is this really performance-sensitive
enough to need to be inline?

Unfortunately, libcpp still uses its own diagnostic output code, so
either we'd need to update that as well, or we'd have to finally make
libcpp be able to use the GCC machinery.

Tom