This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [4.5] [RFC] Move line-map.* to its own library (libloc)


Hello Manuel,

a couple of quick comments.

* Manuel López-Ibáñez wrote on Fri, Nov 14, 2008 at 09:02:06PM CET:
> Index: gcc/Makefile.in
> ===================================================================
> --- gcc/Makefile.in	(revision 141859)
> +++ gcc/Makefile.in	(working copy)
> @@ -291,10 +291,13 @@ PPLINC = @PPLINC@
>  
>  # How to find CLOOG
>  CLOOGLIBS = @CLOOGLIBS@
>  CLOOGINC = @CLOOGINC@
>  
> +LIBLOC = ../libloc/libloc.a
> +LIBINC = -I$(srcdir)/../libloc/include

The naming looks inconsistent.  How about LOCLIB and LOCINC?

>  CPPLIB = ../libcpp/libcpp.a
>  CPPINC = -I$(srcdir)/../libcpp/include
>  
>  # Where to find decNumber
>  enable_decimal_float = @enable_decimal_float@
> @@ -855,13 +858,13 @@ C_COMMON_H = c-common.h $(SPLAY_TREE_H) 
>  C_PRAGMA_H = c-pragma.h $(CPPLIB_H)
>  C_TREE_H = c-tree.h $(C_COMMON_H) $(TOPLEV_H) $(DIAGNOSTIC_H)
>  SYSTEM_H = system.h hwint.h $(srcdir)/../include/libiberty.h \
>  	$(srcdir)/../include/safe-ctype.h $(srcdir)/../include/filenames.h
>  PREDICT_H = predict.h predict.def
> -CPPLIB_H = $(srcdir)/../libcpp/include/line-map.h \
> -	$(srcdir)/../libcpp/include/cpplib.h
> -INPUT_H = $(srcdir)/../libcpp/include/line-map.h input.h
> +LIBLOC_H = $(srcdir)/../libloc/include/line-map.h 

That should be named LINE_MAP_H, for sanity.

> +CPPLIB_H = $(LIBLOC_H) $(srcdir)/../libcpp/include/cpplib.h
> +INPUT_H = $(LIBLOC_H) input.h
>  DECNUM_H = $(DECNUM)/decContext.h $(DECNUM)/decDPD.h $(DECNUM)/decNumber.h \
>  	$(DECNUMFMT)/decimal32.h $(DECNUMFMT)/decimal64.h \
>  	$(DECNUMFMT)/decimal128.h $(DECNUMFMT)/decimal128Local.h
>  MKDEPS_H = $(srcdir)/../libcpp/include/mkdeps.h
>  SYMTAB_H = $(srcdir)/../libcpp/include/symtab.h $(OBSTACK_H)
> @@ -910,19 +913,19 @@ ALL_CPPFLAGS = $(INCLUDES) $(CPPFLAGS)
>  # Build and host support libraries.
>  LIBIBERTY = ../libiberty/libiberty.a
>  BUILD_LIBIBERTY = $(build_libobjdir)/libiberty/libiberty.a
>  
>  # Dependencies on the intl and portability libraries.
> -LIBDEPS= $(CPPLIB) $(LIBIBERTY) $(LIBINTL_DEP) $(LIBICONV_DEP) $(LIBDECNUMBER)
> +LIBDEPS= $(CPPLIB) $(LIBLOC) $(LIBIBERTY) $(LIBINTL_DEP) $(LIBICONV_DEP) $(LIBDECNUMBER)
>  
>  # Likewise, for use in the tools that must run on this machine
>  # even if we are cross-building GCC.
>  BUILD_LIBDEPS= $(BUILD_LIBIBERTY)
>  
>  # How to link with both our special library facilities
>  # and the system's installed libraries.
> -LIBS = @LIBS@ $(CPPLIB) $(LIBINTL) $(LIBICONV) $(LIBIBERTY) $(LIBDECNUMBER) \
> +LIBS = @LIBS@ $(CPPLIB) $(LIBLOC) $(LIBINTL) $(LIBICONV) $(LIBIBERTY) $(LIBDECNUMBER) \
>         $(GMPLIBS) $(CLOOGLIBS) $(PPLLIBS)
>  
>  # Any system libraries needed just for GNAT.
>  SYSLIBS = @GNAT_LIBEXC@
>  
> @@ -948,11 +951,11 @@ BUILD_ERRORS = build/errors.o
>  # -I$(@D) and -I$(srcdir)/$(@D) cause the subdirectory of the file
>  # currently being compiled, in both source trees, to be examined as well.
>  # libintl.h will be found in ../intl if we are using the included libintl.
>  INCLUDES = -I. -I$(@D) -I$(srcdir) -I$(srcdir)/$(@D) \
>  	   -I$(srcdir)/../include @INCINTL@ \
> -	   $(CPPINC) $(GMPINC) $(DECNUMINC) \
> +	   $(CPPINC) $(LIBINC) $(GMPINC) $(DECNUMINC) \
>  	   $(PPLINC) $(CLOOGINC)
>  
>  .c.o:
>  	$(CC) -c $(ALL_CFLAGS) $(ALL_CPPFLAGS) $< $(OUTPUT_OPTION)
>  
> @@ -1336,11 +1339,11 @@ ALL_HOST_OBJS = $(GCC_OBJS) $(C_OBJS) $(
>    $(foreach v,$(CONFIG_LANGUAGES),$($(v)_OBJS)) \
>    $(COLLECT2_OBJS) $(EXTRA_GCC_OBJS) \
>    mips-tfile.o mips-tdump.o \
>    $(PROTO_OBJS) $(GCOV_OBJS) $(GCOV_DUMP_OBJS)
>  
> -BACKEND = main.o @TREEBROWSER@ libbackend.a $(CPPLIB) $(LIBDECNUMBER)
> +BACKEND = main.o @TREEBROWSER@ libbackend.a $(CPPLIB) $(LIBLOC) $(LIBDECNUMBER)
>  
>  MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \
>   insn-output.c insn-recog.c insn-emit.c insn-extract.c insn-peep.c \
>   insn-attr.h insn-attrtab.c insn-opinit.c insn-preds.c insn-constants.h \
>   tm-preds.h tm-constrs.h \

> Index: libloc/configure
> ===================================================================
> --- libloc/configure	(revision 0)
> +++ libloc/configure	(revision 0)

Please do not post changes to generated files like configure; in many
directories (the toplevel one, for example), Makefile.in files are
generated, too.

> --- libloc/Makefile.in	(revision 0)
> +++ libloc/Makefile.in	(revision 0)

FWIW, I don't see why you shouldn't be able to use automake for libloc.
The Makefile.am file could probably be

noinst_LIBRARIES = libloc.a
libloc_a_SOURCES = line-map.c
ACLOCAL_AMFLAGS = -I ../config
# It is not possible to get LOCALEDIR defined in config.h because
# the value it needs to be defined to is only determined in the
# Makefile.  Hence we do this instead.
localedir.h: localedir.hs; @true
localedir.hs: Makefile
	echo "#define LOCALEDIR \"$(localedir)\"" > localedir.new
	$(srcdir)/../move-if-change localedir.new localedir.h
	echo timestamp > localedir.hs

CATALOGS = $(patsubst %,po/%,@CATALOGS@)

WARN_CFLAGS = @WARN_CFLAGS@ @WARN_PEDANTIC@ @WERROR@
INCLUDES = -I$(srcdir) -I. -I$(srcdir)/../include @INCINTL@ \
	-I$(srcdir)/include
AM_CFLAGS = $(WARN_CFLAGS) $(INCLUDES)

# It is not possible to get LOCALEDIR defined in config.h because
# the value it needs to be defined to is only determined in the
# Makefile.  Hence we do this instead.
localedir.h: localedir.hs; @true
localedir.hs: Makefile
	echo "#define LOCALEDIR \"$(localedir)\"" > localedir.new
	$(srcdir)/../move-if-change localedir.new localedir.h
	echo timestamp > localedir.hs

update-po: $(CATALOGS:.gmo=.pox)

# Implicit rules and I18N

# N.B. We do not attempt to copy these into $(srcdir).
.po.gmo:
	-test -d po || mkdir po
	$(GMSGFMT) --statistics -o $@ $<

# The new .po has to be gone over by hand, so we deposit it into
# build/po with a different extension.
# If build/po/$(PACKAGE).pot exists, use it (it was just created),
# else use the one in srcdir.
.po.pox:
	-test -d po || mkdir po
	$(MSGMERGE) $< `if test -f po/$(PACKAGE).pot; \
	                then echo po/$(PACKAGE).pot; \
	                else echo $(srcdir)/po/$(PACKAGE).pot; fi` -o $@

# Rule for regenerating the message template.
$(PACKAGE).pot: po/$(PACKAGE).pot
po/$(PACKAGE).pot: $(libloc_a_SOURCES)
	-test -d $(srcdir)/po || mkdir $(srcdir)/po
	$(XGETTEXT) --default-domain=$(PACKAGE) \
	  --keyword=_ --keyword=N_ \
	  --keyword=cpp_error:3 --keyword=cpp_errno:3 \
	  --keyword=cpp_error_with_line:5 \
	  --keyword=SYNTAX_ERROR --keyword=SYNTAX_ERROR2 \
	  --copyright-holder="Free Software Foundation, Inc." \
	  --msgid-bugs-address="http://gcc.gnu.org/bugs.html"; \
	  --language=c -o po/$(PACKAGE).pot.tmp $^
	sed 's:$(srcdir)/::g' <po/$(PACKAGE).pot.tmp >po/$(PACKAGE).pot
	rm po/$(PACKAGE).pot.tmp


Cheers,
Ralf


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]