Link to previous discussion: https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00648.html
On Mon, 2016-10-10 at 19:53 +0100, Richard Sandiford wrote: > David Malcolm <dmalc...@redhat.com> writes: > > On Wed, 2016-10-05 at 18:00 +0200, Bernd Schmidt wrote: > > > On 10/05/2016 06:15 PM, David Malcolm wrote: > > > > * errors.c: Use consistent pattern for bconfig.h vs > > > > config.h > > > > includes. > > > > (progname): Wrap with #ifdef GENERATOR_FILE. > > > > (error): Likewise. Add "error: " to message. > > > > (fatal): Likewise. > > > > (internal_error): Likewise. > > > > (trim_filename): Likewise. > > > > (fancy_abort): Likewise. > > > > * errors.h (struct file_location): Move here from read > > > > -md.h. > > > > (file_location::file_location): Likewise. > > > > (error_at): New decl. > > > > > > Can you split these out into a separate patch as well? I'll > > > require > > > more > > > explanation for them and they seem largely independent. > > > > [CCing Richard Sandiford] > > > > The gen* tools have their own diagnostics system, in errors.c: > > > > /* warning, error, and fatal. These definitions are suitable for > > use > > in the generator programs; the compiler has a more elaborate > > suite > > of diagnostic printers, found in diagnostic.c. */ > > > > with file locations tracked using read-md.h's struct file_location, > > rather than location_t (aka libcpp's source_location). > > > > Implementing an RTL frontend by using the RTL reader from read > > -rtl.c > > means that we now need a diagnostics subsystem on the *host* for > > handling errors in RTL files, rather than just on the build > > machine. > > > > There seem to be two ways to do this: > > > > (A) build the "light" diagnostics system (errors.c) for the host > > as > > well as build machine, and link it with the RTL reader there, so > > there > > are two parallel diagnostics subsystems. > > > > (B) build the "real" diagnostics system (diagnostics*) for the > > *build* machine as well as the host, and use it from the gen* > > tools, > > eliminating the "light" system, and porting the gen* tools to use > > libcpp for location tracking. > > > > Approach (A) seems to be simpler, which is what this part of the > > patch > > does. > > > > I've experimented with approach (B). I think it's doable, but it's > > much more invasive (perhaps needing a libdiagnostics.a and a > > build/libdiagnostics.a in gcc/Makefile.in), so I hope this can be > > followup work. > > > > I can split the relevant parts out into a separate patch, but I was > > wondering if either of you had a strong opinion on (A) vs (B) > > before I > > do so? > > (A) sounds fine to me FWIW. And sorry for the slow reply. > > Thanks, > Richard This patch implements approach (A). It updates the error messages to contain "error: " so that they can be handled by DejaGnu. It also adds an "error_at" function to the "light" API. Doing so requires moving the decl of struct file_location to errors.h. gcc/ChangeLog: * Makefile.in (OBJS): Add errors.o. * errors.c: Use consistent pattern for bconfig.h vs config.h includes. (progname): Wrap with #ifdef GENERATOR_FILE. (error): Likewise. Add "error: " to message. (fatal): Likewise. (internal_error): Likewise. (trim_filename): Likewise. (fancy_abort): Likewise. * errors.h (struct file_location): Move here from read-md.h. (file_location::file_location): Likewise. (error_at): New decl. * read-md.h (struct file_location): Move to errors.h. (file_location::file_location): Likewise. Include errors.h. --- gcc/Makefile.in | 1 + gcc/errors.c | 23 +++++++++++++++++------ gcc/errors.h | 14 ++++++++++++++ gcc/read-md.h | 14 +------------- 4 files changed, 33 insertions(+), 19 deletions(-) diff --git a/gcc/Makefile.in b/gcc/Makefile.in index fa54215..c265893 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1266,6 +1266,7 @@ OBJS = \ dwarf2cfi.o \ dwarf2out.o \ emit-rtl.o \ + errors.o \ et-forest.o \ except.o \ explow.o \ diff --git a/gcc/errors.c b/gcc/errors.c index 468384c..8df94ab 100644 --- a/gcc/errors.c +++ b/gcc/errors.c @@ -21,18 +21,21 @@ along with GCC; see the file COPYING3. If not see in the generator programs; the compiler has a more elaborate suite of diagnostic printers, found in diagnostic.c. */ -#ifdef HOST_GENERATOR_FILE -#include "config.h" -#define GENERATOR_FILE 1 -#else +/* This file is compiled twice: once for the generator programs + once for the compiler. */ +#ifdef GENERATOR_FILE #include "bconfig.h" +#else +#include "config.h" #endif #include "system.h" #include "errors.h" /* Set this to argv[0] at the beginning of main. */ +#ifdef GENERATOR_FILE const char *progname; +#endif /* #ifdef GENERATOR_FILE */ /* Starts out 0, set to 1 if error is called. */ @@ -55,13 +58,15 @@ warning (const char *format, ...) /* Print an error message - we keep going but the output is unusable. */ +#ifdef GENERATOR_FILE + void error (const char *format, ...) { va_list ap; va_start (ap, format); - fprintf (stderr, "%s: ", progname); + fprintf (stderr, "%s: error: ", progname); vfprintf (stderr, format, ap); va_end (ap); fputc ('\n', stderr); @@ -69,6 +74,7 @@ error (const char *format, ...) have_error = 1; } +#endif /* #ifdef GENERATOR_FILE */ /* Fatal error - terminate execution immediately. Does not return. */ @@ -78,7 +84,7 @@ fatal (const char *format, ...) va_list ap; va_start (ap, format); - fprintf (stderr, "%s: ", progname); + fprintf (stderr, "%s: error: ", progname); vfprintf (stderr, format, ap); va_end (ap); fputc ('\n', stderr); @@ -87,6 +93,8 @@ fatal (const char *format, ...) /* Similar, but say we got an internal error. */ +#ifdef GENERATOR_FILE + void internal_error (const char *format, ...) { @@ -127,8 +135,11 @@ trim_filename (const char *name) /* "Fancy" abort. Reports where in the compiler someone gave up. This file is used only by build programs, so we're not as polite as the version in diagnostic.c. */ + void fancy_abort (const char *file, int line, const char *func) { internal_error ("abort in %s, at %s:%d", func, trim_filename (file), line); } + +#endif /* #ifdef GENERATOR_FILE */ diff --git a/gcc/errors.h b/gcc/errors.h index a6973f3..ebafa77 100644 --- a/gcc/errors.h +++ b/gcc/errors.h @@ -28,8 +28,22 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_ERRORS_H #define GCC_ERRORS_H +/* Records a position in the file. */ +struct file_location { + file_location () {} + file_location (const char *, int, int); + + const char *filename; + int lineno; + int colno; +}; + +inline file_location::file_location (const char *filename_in, int lineno_in, int colno_in) +: filename (filename_in), lineno (lineno_in), colno (colno_in) {} + extern void warning (const char *, ...) ATTRIBUTE_PRINTF_1 ATTRIBUTE_COLD; extern void error (const char *, ...) ATTRIBUTE_PRINTF_1 ATTRIBUTE_COLD; +extern void error_at (file_location, const char *, ...) ATTRIBUTE_PRINTF_2 ATTRIBUTE_COLD; extern void fatal (const char *, ...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF_1 ATTRIBUTE_COLD; extern void internal_error (const char *, ...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF_1 ATTRIBUTE_COLD; extern const char *trim_filename (const char *); diff --git a/gcc/read-md.h b/gcc/read-md.h index 27fc9c2..8910b75 100644 --- a/gcc/read-md.h +++ b/gcc/read-md.h @@ -21,19 +21,7 @@ along with GCC; see the file COPYING3. If not see #define GCC_READ_MD_H #include "obstack.h" - -/* Records a position in the file. */ -struct file_location { - file_location () {} - file_location (const char *, int, int); - - const char *filename; - int lineno; - int colno; -}; - -inline file_location::file_location (const char *filename_in, int lineno_in, int colno_in) -: filename (filename_in), lineno (lineno_in), colno (colno_in) {} +#include "errors.h" /* Holds one symbol or number in the .md file. */ struct md_name { -- 1.8.5.3