gengtype improvements for plugins. patch 3/N [input_file]
Laurynas Biveinis
laurynas.biveinis@gmail.com
Fri Sep 3 08:47:00 GMT 2010
Again, I assume that this_file and system_h_file will be used
somewhere else outside gengtype.c, is that correct?
> (get_file_srcdir_relative_path, get_file_langdir): Changed formal
> to input_file* in function declarations.
"Change type of the argument to input_file*"
> (read_input_line): Updated comment. Need no more the horrible
> lang_bitmap space kludge!
Drop the "whys" :) Just "Update comment".
> * gengtype.h: Added new input_file type and upgraded declarations.
> Moved many definitions from gengtype.c & declared several of its
> functions.
What is an upgraded declaration?
> * gengtype-lex.l: Added include of errors.h
Include errors.h.
+/* Variable length structure representing an input file. An hash table
".. A hash..."
+ ensure uniqueness for a given input file name. */
+struct input_file_st
+{
+ int inpmagic; /* Always INPUT_FILE_MAGIC. */
Does this magic buy us anything? If you were debugging a memory
corruption, I guess Valgrind would have caught it as well (strictly
speaking, not necessarily - but quite likely). Now that you are done
with debugging, might as well remove it. :)
+#define INPUT_FILE_MAGIC 924599103 /* 0x371c433f */
Please move outside of struct, define in hex, drop the dec.
+ struct outf* inpoutf; /* Cached corresponding output file, computed
+ in get_output_file_with_visibility. */
+ lang_bitmap inpbitmap; /* The set of languages using this file. */
+ char inpname[1]; /* a flexible array, ended by a null char. */
+};
+/* Table of all input files, and its number. */
and its size.
+extern input_file** gt_files;
+extern size_t num_gt_files;
+#if ENABLE_CHECKING
+ if (inpf->inpmagic != INPUT_FILE_MAGIC)
+ fatal("corrupted input file %p -bad magic %d",
+ (const void*) inpf, inpf->inpmagic);
+#endif
If this is to be kept (see my comments about magic above), then
refactor it out to some function and renamve input_file_name to
get_input_file_name for symmetry with get/set_lang_bitmap. Otherwise,
inline input_file_name, get_lang_bitmap, set_lang_bitmap as they
become one-liners if checking is removed.
+/* A file location, mostly for error messages. */
struct fileloc {
- const char *file;
+ input_file *ifile;
int line;
};
.IMHO a rename not necessary (also would help to reduce the patch size).
--- ../gengtype-gcc-02/gengtype-lex.l 2010-08-29 11:04:49.000000000 +0200
+++ gcc/gengtype-lex.l 2010-08-29 11:07:30.000000000 +0200
...
+#include "errors.h" /* for fatal */
...
- lexer_line.file = fname;
+ lexer_line.ifile = input_file_by_name (fname);
You are not using fatal here. Either remove the include or adjust the
comment why this is needed. Same for gengtype-parse.c.
--- ../gengtype-gcc-02/gengtype.c 2010-08-29 08:17:58.000000000 +0200
+++ gcc/gengtype.c 2010-08-29 11:25:37.000000000 +0200
+/* The plugin input files and their number; in that case only a single
+ file is produced.
In what case?
+/****************************************************************
+ * Manage input files.
+ ****************************************************************/
Follow the style used elsewhere for source file sections (IIRC a ^L
and a single line comment)
+input_file*
+input_file_by_name (const char* name)
Needs a comment.
+ /* Fresh input file. */
New input file.
-/* For F a filename, return the relative path to F from $(srcdir) if the
- latter is a prefix in F, NULL otherwise. */
+/* For F an input_file, return the relative path to F from $(srcdir)
+ if the latter is a prefix in F, NULL otherwise. */
static const char *
-get_file_srcdir_relative_path (const char *f)
+get_file_srcdir_relative_path (const input_file *inpf)
/* For INPF, return the relative path to INPF from $(srcdir) if the
latter ... */
Similar in further comments: no need to describe what inpf is, comment
should also refer to INPF not F.
> output_name = "gtype-desc.c";
If you made definitions for system.h and gengtype.c, might as well
provide one for gtype-desc.c too.
I like the patch, but it IMHO it needs further work.
--
Laurynas
More information about the Gcc-patches
mailing list