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: gengtype improvements for plugins, completed! patch 6/N [wstate]


Below are some comments but I did not read the patch very thoroughly.

Does the state dump output completely subsume the gengtype debugging
output? If yes, then it's a good idea to drop the latter.

+enum state_token_en
+{
+  STOK_NONE,			/* never used */
+  STOK_INTEGER,			/* integer token */

Comment indentation.

+/* Our token lexer is heavily inspired by MELT's lexer, and share some
+   code with the file gcc/melt-runtime.c of the GCC MELT branch!  We
+   really want the gengtype state to be easily parsable by MELT. */
+static struct state_token_st *
+read_a_state_token (void)

Can you break up this function to several smaller ones?

+static void
+write_state_type_option (options_p current)

This and two following function need their own header comments.

@@ -3792,6 +3792,9 @@ GTFILES_H = $(subst /,-, \
 GTFILES_LANG_H = $(patsubst [%], gtype-%.h, $(filter [%], $(GTFILES)))
 ALL_GTFILES_H := $(sort $(GTFILES_H) $(GTFILES_LANG_H))

+## Common flags to gengtype. Used for verbosity.
+GENGTYPE_FLAGS = -v -v
+
 # $(GTFILES) may be too long to put on a command line, so we have to
 # write it out to a file (taking care not to do that in a way that
 # overflows a command line!) and then have gengtype read the file in.

Please do not be verbose by default.

-	$(RUN_GEN) build/gengtype$(build_exeext) -S $(srcdir) -I gtyp-input.list
+# The old way to generate in one shot.
+#	$(RUN_GEN) build/gengtype$(build_exeext) -S $(srcdir) -I gtyp-input.list

Looks identical. Also, if something is "old", please just remove it
and not comment out, or it will bit-rot very fast.

> There is still some small point missing, essentially a Makefile.in
> fix. What should we do of the generated state file of gengtype?
> Where should it be installed? ÂI was thinking of the plugins
> directory, but there might be some better idea.
>
> Also, how should the gengtype program be installed? Perhaps it should
> be named gcc-gengtype? ÂI need help on these minor issues!

I can only offer some uninformed speculation on the topic -

What is the status of plugins with respect to cross-compilation? I
guess it is possible to cross-compile a plugin to be run on GCC on
different architecture?

Gengtype currently is a build tool. Gengtype after your patches will
be both build (to build GCC itself and to produce state dump for later
usage) and plugin-build (to read the state dump for building a
plugin). Now build and plugin-build are not necessarily the same, in
fact as I see it, it needs not to be any of build, host, target system
of a GCC build!

I guess in practice you will have to limit gengtype to either build or
host of a GCC build and that would determine how it gets installed
(but we don't install build tools, do we?).

I agree that gcc-gengtype is a good name.

-- 
Laurynas


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