This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] Factor common code for C-family and Fortran preprocessing (2nd try)
On Tue, 28 Sep 2010, FX wrote:
> 2010-09-28 Francois-Xavier Coudert <fxcoudert@gcc.gnu.org>
>
> * cppbuiltin.c: New file.
> * cppbuiltin.h: New file.
> * Makefile.in: Take into account ccpbuiltin.{h,c}.
cppbuiltin not ccpbuiltin.
> gcc/c-family/
>
> 2010-09-28 Francois-Xavier Coudert <fxcoudert@gcc.gnu.org>
>
> * c-cppbuiltin.c (define__GNUC__): Remove.
> (c_cpp_builtins): Call functions from ccpbuiltin.c instead
Likewise.
> Index: gcc/cppbuiltin.c
> ===================================================================
> --- gcc/cppbuiltin.c (revision 0)
> +++ gcc/cppbuiltin.c (revision 0)
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "tm.h"
> +#include "tree.h"
> +#include "version.h"
> +#include "flags.h"
> +#include "output.h"
> +#include "toplev.h"
> +#include "tm_p.h" /* For TARGET_CPU_CPP_BUILTINS & friends. */
> +#include "target.h"
> +#include "cpp-id-data.h"
> +#include "cppbuiltin.h"
Please check whether all these includes are actually needed. I don't see
any actual use of TARGET_CPU_CPP_BUILTINS in this file, so the comment
referencing it seems wrong, for example.
> +
> +
> +void
> +define_builtins_for_compilation_flags (cpp_reader *pfile)
All functions need comments explaining them. Especially important when
they form part of the public interface to this file.
"builtins" would generally be interpreted as meaning built-in functions -
I think you should use names making clear it is *macros* involved.
> +void
> +define_builtin_for_lp64 (cpp_reader *pfile)
Why "builtin" singular?
> +void
> +define_builtins_for_basic_typesizes (cpp_reader *pfile)
Why "typesizes" rather than "type_sizes" (or
"definebuiltinsforbasictypesizes")?
> +extern void define__GNUC__ (cpp_reader *);
> +extern void define_builtins_for_compilation_flags (cpp_reader *);
> +extern void define_builtin_for_lp64 (cpp_reader *);
> +extern void define_builtins_for_basic_typesizes (cpp_reader *);
Do you actually need all these separate functions, and potentially more in
future for other groups of macros? Or will all of them always be called
together, so you could have a single function
define_language_independent_builtin_macros?
Separate static functions in cppbuiltin.c seems reasonable - but I'd hope
the public interface could be smaller and so more stable, with the one
public function calling all the static ones.
--
Joseph S. Myers
joseph@codesourcery.com