Re: [RFA] Move cpplib to the toplevel

[Zack Weinberg <zack at codesourcery dot com>]

Paolo Bonzini <bonzini@gnu.org> writes:

> Changes WRT the previous version:
>
> - I left hashtable.h in cpplib
> - I thus avoided messing with boehm-gc and libiberty
> - but, I have to link in libcpp.a in $(BACKEND) for all front-ends.
> - I added a note about input charsets being disabled.
> - minor cleanups due to tree-ssa merge.
>
> Bootstrapped/regtested i686-pc-linux-gnu, no new regression, all
> working languages.

This is very nice.  I have some structural changes that I'd like you
to make, though.

First, as long as we're moving the files around, I'd like to rename
them, like so:

old name        new name
--------        --------
cppcharset.c    charset.c
cpperror.c      errors.c
cppexp.c        expr.c
cppfiles.c      files.c
cpphash.c       identifiers.c
cppinit.c       init.c
cpplex.c        lex.c
cpplib.c        directives.c
cppmacro.c      macro.c
cpppch.c        pch.c
cpptrad.c       traditional.c
cppucnid.h      ucnid.h
cppucnid.pl     ucnid.pl
cppucnid.tab    ucnid.tab
hashtable.c     symtab.c
line-map.c      line-map.c
mkdeps.c        mkdeps.c

cpphash.h       internal.h
cpplib.h        include/cpplib.h
hashtable.h     include/symtab.h
line-map.h      include/line-map.h
mkdeps.h        include/mkdeps.h

You'll note that cpphash.h does not go in cpplib/include, but the rest
of the headers do.  That's because cpphash.h is not supposed to be
included by any other .c file than the ones you've moved.  The -I
switch for the gcc directory should be -I$(srcdir)/../libcpp/include
(and to match GMP and Banshee, give it a variable).  I know there are
one or two other source files that do include cpphash.h.  They should
have to jump through special hoops, i.e. #include "../libcpp/internal.h",
to remind us that they need to be fixed.

I was hoping to get some code reorganization done at the same time as
this move, but let's not bother - we don't want to slow down this
process any more than it already is.

Second, use of Automake here is inappropriate.  It drags in a whole
pile of compatibility concerns, and furthermore it generates rules I
actively don't want, such as installation rules.  cpplib is not to be
installed anywhere until we're prepared to make much stronger
guarantees about its API and ABI than we are right now.  For similar
reasons I don't even want to consider building a shared library.  So
please write a simple hand-coded Makefile.in - it won't be much longer
than the Makefile.am you already did.

Third, please audit configure.in and system.h and make sure that
everything in them is being used.  It looks like you copied a lot of
stuff from the gcc versions of those files without checking.

Fourth, this can be addressed after your patch goes in, but I don't
see any of the necessary goo to enable NLS and specify the message
domain for cpplib's errors.

zw