On Mon, 2016-11-28 at 14:47 +0100, Bernd Schmidt wrote:
> Been looking at this off and on, and I'm still not sure I entirely
> get 
> it - sorry.
> 
> On 11/11/2016 10:15 PM, David Malcolm wrote:
> > > > 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.
> 
> So, there are two things that bother me about this patch description:
>   - The host already has the full diagnostic subsystem.

Maybe I worded this poorly.

I meant to say:

"we now need
  ((a diagnostics subsystem for handling errors in RTL files)
   on the *host*),
rather than just on the build machine."

rather than:

"we now need a
  ((diagnostics subsystem on the *host*)
   for handling
errors in RTL files),
 rather than just on the build machine."

if that distinction makes sense.  Clearly we already have a diagnostics
subsystem on the host; what this patch is adding is the separate, rtl-s
pecific diagnostic subsystem to cc1 on the host.

> The fact that
>     you're commenting out some of the functions in errors.c suggests
>     that errors.c is conflicting with the full one.

It doesn't conflict: C++ overloading allows both to co-exist.  However
I wanted to make sure that we don't accidentally use the RTL-specific
error-handling within other parts of the compiler.

>   - We already compile errors.c for both build and host.

Aha, yes, we do, it's linked into gengtype on the host to allow plugins
to support GTY.  The patch adds it to OBJS so that it is available
within cc1.

> Is there a problem with using both the full and the light errors
> system 
> for read-rtl, as available? Mismatches in function signatures or 
> something like this?

As noted above, C++ overloading allows this.

> > -#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"
> 
> The Makefile still has a HOST_GENERATOR_FILE definition for errors.c 
> after this.

Will remove.

Reply via email to