Re: [patch, fortran] PR18428: use libcpp for preprocessing (review of C/C++ and fortran maintainers needed)

["Daniel Franke" <franke dot daniel at gmail dot com>]

> The Fortran parts are OK provided you answer the questions
> below and:
>   -- provide a patch for the news page
> (http://gcc.gnu.org/gcc-4.4/changes.html),
>   -- rewrite the doc for preprocessing
> (http://gcc.gnu.org/onlinedocs/gfortran/Preprocessing-and-conditional-compilation.html);
> I suggest you add a link to CPP options there.

FX,
thanks for the review. I already prepared the docs, but didn't submit them yet.
I'm currently out of internet, I'll answer some of your questions
shortly before going through more thoroughly when I'm back online.


> > @@ -354,6 +357,9 @@ gfc_post_options (const char **pfilename
> >   if (gfc_option.flag_all_intrinsics)
> >     gfc_option.warn_nonstd_intrinsics = 0;
> >
> > +  gfc_cpp_post_options ();
> > +
> > +/* FIXME: return gfc_cpp_preprocess_only (); */
> >   return false;
> >  }
> >
>
>  I don't understand this FIXME: why should we do that? And why isn't it done
> here?

I don't have the code around, but IIRC: the return value sets a
no_backend (or similar) variable in the driver. If 'false' is returned
the backend is initialized, otherwise not. In the latter case,
something somewhere crashed and gave an ICE.


> > +  /* Initialize CPP built-ins; '1' corresponds to 'flag_hosted'
> > +     in C, defines __STDC_HOSTED__?!  */
> > +  cpp_init_builtins (cpp_in, 1);
> >
>
>  I don't think we want to define __STDC_HOSTED__. On the other hand, does
> your patch still define the CPU, OS and OBJFMT builtins? (macros
> TARGET_CPU_CPP_BUILTINS, TARGET_OS_CPP_BUILTINS and
> TARGET_OBJFMT_CPP_BUILTINS) I think we should. Other macros that we used to
> emit and I think we will now miss include: __VERSION__,
> [...] macros (which are I think potentially useful).

None of these are set. I went with the docs and only implemented those
mentioned. If we want all macros defined by gcc, we probably can
re-use their code.


> > +  /* Supply enough spaces to put this token in its original column,
> > +     one space per column greater than 2, since scan_translation_unit
> > +     will provide a space if PREV_WHITE.  Don't bother trying to
> > +     reconstruct tabs; we can't get it right in general, and nothing
> > +     ought to care.  Some things do care; the fault lies with them.  */
>
>  Can you expand a bit more here? What kind of thing will change or be broken
> exactly?

Errr, please ask the original author. I think, this was copied
verbatim from c-ppoutput.c :)


Be back soon

    Daniel