===================================================================
@@ -25,14 +25,117 @@
represented by a bitmap. */
typedef unsigned lang_bitmap;
-/* A file position, mostly for error messages.
- The FILE element may be compared using pointer equality. */
+/* Variable length structure representing an input file. A hash table
+ ensure uniqueness for a given input file name. The only function
+ allocating input_file-s is input_file_by_name. */
/* Variable length structure representing an input file. Each unique given
file name has one and only one variable of this type, stored in a hash
table input_file_htab in gengtype.c. */
===================================================================
@@ -24,7 +24,9 @@ along with GCC; see the file COPYING3. If not see
%{
#include "bconfig.h"
#include "system.h"
+#include "errors.h" /* for fatal, needed by gengtype.h */
+
#define malloc xmalloc
#define realloc xrealloc
@@ -202,7 +204,7 @@ yybegin (const char *fname)
perror (fname);
exit (1);
}
- lexer_line.file = fname;
+ lexer_line.file = input_file_by_name (fname);
lexer_line.line = 1;
}
We have discussed this before but I still do not follow. This patch
does not add include of gengtype.h, so if it worked previsouly without
#include "errors.h", it should still work. Likewise for
gengtype-parse.c.
@@ -708,6 +710,7 @@ static type_p
type (options_p *optsp, bool nested)
{
const char *s;
+ static int anonymous_count; /* To generate unique pseudo-identifiers! */
*optsp = 0;
switch (token ())
{
@@ -750,7 +753,32 @@ type (options_p *optsp, bool nested)
if (token () == ID)
s = advance ();
else
- s = xasprintf ("anonymous:%s:%d", lexer_line.file, lexer_line.line);
+ {
+ /* We don't want to wire in the source directory (because
+ in plugin mode, the source directory can be unavailable
+ since gengtype has read its state). So if the input is
+ from GCC source directory, we use its relative path to
+ build an anonymous unique tag. */
+ const char* relp = get_file_srcdir_relative_path (lexer_line.file);
+ anonymous_count++;
+ if (relp)
+ {
+ /* The input file is a GCC source file, we use a double
+ colon after anonymous. To be sure s is truly unique,
+ we also use anonymous_count. */
+ s = xasprintf ("anonymous::%s:%d::%d",
+ relp, lexer_line.line, anonymous_count);
+ }
+ else
+ {
+ /* The input file is outside of GCC source tree, we use
+ a single colon after anonymous. To be sure s is
+ truly unique, we also use anonymous_count. */
+ s = xasprintf ("anonymous:%s:%d::%d",
+ get_input_file_name (lexer_line.file),
+ lexer_line.line, anonymous_count);
+ }
+ }
/* Unfortunately above GTY_TOKEN check does not capture the
typedef struct_type GTY case. */
@@ -787,7 +815,19 @@ type (options_p *optsp, bool nested)
if (token () == ID)
s = advance ();
else
- s = xasprintf ("anonymous:%s:%d", lexer_line.file, lexer_line.line);
+ {
+ /* Again, we don't want to wire in the GCC source tree
+ directory. */
+ const char* relp = get_file_srcdir_relative_path (lexer_line.file);
+ anonymous_count++;
+ if (relp)
+ s = xasprintf ("anonymous::%s:%d::%d",
+ relp, lexer_line.line, anonymous_count);
+ else
+ s = xasprintf ("anonymous:%s:%d::%d",
+ get_input_file_name (lexer_line.file),
+ lexer_line.line, anonymous_count);
+ }
if (token () == '{')
consume_balanced ('{', '}');
Separate patch please.
2010/10/20 Basile Starynkevitch <basile@starynkevitch.net>:
>
> Hello all,
>
> References:
> http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01742.html
> http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01536.html
> http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01452.html
>
> and others linked from them.
>
> After having separately commited the removal of location_s and having
> taken account Laurynas's comments
> http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01452.html I am attaching
> a patch on gengtype adding a real input_file structure relative to
To shortly sum up the review (details below): - The patch should not contain any seemingly random code shuffling, reformatting and whitespace changes - I found two or three parts that should be independent submissions - I'm afraid I am going to ask you to drop the magic. I do believe that these days the tool support at least on Linux is good enough to keep the source code concise. Thanks, Laurynas -static const char *get_file_basename (const char *); -static const char *get_file_realbasename (const char *); -static const char *get_file_srcdir_relative_path (const char *); +static const char * get_file_basename (const input_file *); +static const char * get_file_realbasename (const input_file *); static const char *get_file_basename ... static const char *get_file_realbasename ... -dbgprint_count_type_at (const char *fil, int lin, const char *msg, type_p t) +dbgprint_count_type_at (const char *fil, int lin, const char*msg, type_p t) Please drop this chunk. if (bmap & curlangs) - error_at_line (&epos, - "file %s specified more than once " + error_at_line (&epos, "file %s specified more than once " "for language %s", line, - langno == - 0 ? "(all)" : lang_dir_names[langno - - 1]); + langno == 0 + ? "(all)" : lang_dir_names[langno - 1]); Is this only formatting change? If yes, please drop. @@ -886,7 +839,7 @@ note_variable (const char *s, type_p t, options_p /* Most-general structure field creator. */ static pair_p create_field_all (pair_p next, type_p type, const char *name, options_p opt, - const char *file, int line) + const input_file* inpf, int line) { pair_p field; GNU coding standards say the star sticks to the parameter name, so const input_file *inpf, int line) @@ -1080,7 +1033,7 @@ adjust_field_rtx_def (type_p t, options_p ARG_UNUS mem_attrs_tp = create_pointer (find_structure ("mem_attrs", 0)); reg_attrs_tp = create_pointer (find_structure ("reg_attrs", 0)); basic_block_tp = create_pointer (find_structure ("basic_block_def", 0)); - constant_tp = + constant_tp = create_pointer (find_structure ("constant_descriptor_rtx", 0)); scalar_tp = &scalar_nonchar; /* rtunion int */ @@ -1350,7 +1303,7 @@ adjust_field_type (type_p t, options_p opt) if (params[num] != NULL) error_at_line (&lexer_line, "duplicate `%s' option", opt->name); if (!ISDIGIT (opt->name[5])) - params[num] = + params[num] = create_pointer (CONST_CAST2 (type_p, const char *, opt->info)); else params[num] = CONST_CAST2 (type_p, const char *, opt->info); @@ -1395,7 +1348,7 @@ static void set_gc_used (pair_p); static void process_gc_options (options_p opt, enum gc_used_enum level, int *maybe_undef, - int *pass_param, int *length, int *skip, + int *pass_param, int *length, int *skip, type_p *nested_ptr) { options_p o; @@ -1592,10 +1546,10 @@ oprintf (outf_p o, const char *format, ...) size_t new_len = o->buflength; if (new_len == 0) new_len = 1024; - do + do { new_len *= 2; - } + } while (o->bufused + slength >= new_len); o->buf = XRESIZEVEC (char, o->buf, new_len); o->buflength = new_len; Here the only change is an extra space at the end of the line. Please drop all these chunks. @@ -1653,8 +1607,9 @@ open_base_files (void) components skipped. */ static const char * -get_file_realbasename (const char *f) +get_file_realbasename (const input_file *inpf) { + const char* f = get_input_file_name (inpf); const char *f = ... @@ -1663,26 +1618,28 @@ static const char * /* For F a filename, 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) +const char * +get_file_srcdir_relative_path (const input_file *inpf) { - if (strlen (f) > srcdir_len - && IS_DIR_SEPARATOR (f[srcdir_len]) - && memcmp (f, srcdir, srcdir_len) == 0) - return f + srcdir_len + 1; + const char* fname = get_input_file_name (inpf); + if (strlen (fname) > srcdir_len + && IS_DIR_SEPARATOR (fname[srcdir_len]) + && memcmp (fname, srcdir, srcdir_len) == 0) + return fname + srcdir_len + 1; else return NULL; } The header comment should refer to INPF, not F const char *fname = Please use strncmp instead of memcmp. -get_file_basename (const char *f) +get_file_basename (const input_file *inpf) { - const char *srcdir_path = get_file_srcdir_relative_path (f); + const char * srcdir_path = get_file_srcdir_relative_path (inpf); const char *srcdir_path = ... -get_output_file_with_visibility (const char *input_file) +get_output_file_with_visibility (input_file* inpf) input_file *inpf @@ -1830,8 +1787,7 @@ outf_p else if (strncmp (basename, "cp", 2) == 0 && IS_DIR_SEPARATOR (basename[2]) && strcmp (basename + 3, "name-lookup.h") == 0) output_name = "gt-cp-name-lookup.h", for_name = "cp/name-lookup.c"; - else if (strncmp (basename, "objc", 4) == 0 - && IS_DIR_SEPARATOR (basename[4]) + else if (strncmp (basename, "objc", 4) == 0 && IS_DIR_SEPARATOR (basename[4]) && strcmp (basename + 5, "objc-act.h") == 0) output_name = "gt-objc-objc-act.h", for_name = "objc/objc-act.c"; else Please drop this chunk. +get_output_file_name (input_file *inpf) { - outf_p o = get_output_file_with_visibility (input_file); + outf_p o = get_output_file_with_visibility (inpf); Watch the spaces around =. @@ -1902,8 +1858,8 @@ is_file_equal (outf_p of) static void close_output_files (void) { + outf_p of; int nbwrittenfiles = 0; - outf_p of; for (of = output_files; of; of = of->next) { Please drop this. @@ -1936,11 +1892,12 @@ close_output_files (void) progname, nbwrittenfiles, of->name, backupname); else if (verbosity_level >= 1) printf ("%s write #%-3d %s\n", progname, nbwrittenfiles, of->name); + if (backupname) free (backupname); Indent the line with free. @@ -1952,11 +1909,12 @@ close_output_files (void) printf ("%s wrote %d files.\n", progname, nbwrittenfiles); } -struct flist + +struct flist { Please drop this change. @@ -1966,8 +1924,8 @@ struct walk_type_data; For structures, given a pointer to the item in 'val'. For misc. pointers, given the item in 'val'. */ -typedef void (*process_field_fn) (type_p f, const struct walk_type_data * p); -typedef void (*func_name_fn) (type_p s, const struct walk_type_data * p); +typedef void (*process_field_fn) (type_p f, const struct walk_type_data *p); +typedef void (*func_name_fn) (type_p s, const struct walk_type_data *p); /* Parameters for write_types. */ Please drop this change. @@ -1999,10 +1957,10 @@ static void write_types_local_process_field static void write_local_func_for_structure (const_type_p orig_s, type_p s, type_p *param); static void write_local (outf_p output_header, - type_p structures, type_p param_structs); + type_p structures, type_p param_structs); Please drop this change. @@ -2041,7 +1999,7 @@ output_mangled_typename (outf_p of, const_type_p t { if (t == NULL) oprintf (of, "Z"); - else + else Please drop. @@ -2057,8 +2015,8 @@ output_mangled_typename (outf_p of, const_type_p t case TYPE_STRUCT: case TYPE_UNION: case TYPE_LANG_STRUCT: - oprintf (of, "%lu%s", (unsigned long) strlen (t->u.s.tag), - t->u.s.tag); + oprintf (of, "%lu%s", (unsigned long) strlen (t->u.s.tag), + t->u.s.tag); break; case TYPE_PARAM_STRUCT: { @@ -2088,7 +2046,7 @@ output_escaped_param (struct walk_type_data *d, co for (p = param; *p; p++) if (*p != '%') oprintf (d->of, "%c", *p); - else + else switch (*++p) { case 'h': @@ -2311,7 +2269,7 @@ walk_type (type_p t, struct walk_type_data *d) oprintf (d->of, "%*sif (%s != NULL) {\n", d->indent, "", d->val); d->indent += 2; oprintf (d->of, "%*ssize_t i%d;\n", d->indent, "", loopcounter); - oprintf (d->of, "%*sfor (i%d = 0; i%d != (size_t)(", d->indent, + oprintf (d->of, "%*sfor (i%d = 0; i%d != (size_t)(", d->indent, "", loopcounter, loopcounter); output_escaped_param (d, length, "length"); oprintf (d->of, "); i%d++) {\n", loopcounter); Please drop. @@ -2639,16 +2597,27 @@ output_type_enum (outf_p of, type_p s) static outf_p get_output_file_for_structure (const_type_p s, type_p *param) { - const char *fn = s->u.s.line.file; + const input_file* fn = NULL; const input_file *fn = NULL; int i; + if (UNION_OR_STRUCT_P (s)) + fn = s->u.s.line.file; + else if (s->kind == TYPE_PARAM_STRUCT) + fn = s->u.param_struct.line.file; Can you submit this separately? + + CHECK_INPUT_FILE_MAGIC (fn); + I'm going to ask you to drop the magic, sorry. @@ -2825,8 +2794,8 @@ static void write_types (outf_p output_header, type_p structures, type_p param_structs, const struct write_types_data *wtd) { + type_p s; int nbfun = 0; /* Count the emitted functions. */ - type_p s; oprintf (output_header, "\n/* %s*/\n", wtd->comment); /* We first emit the macros and the declarations. Functions' code is @@ -2931,12 +2900,9 @@ write_types (outf_p output_header, type_p structur } } else - { - /* Structure s is not possibly pointed to, so can be ignored. */ DBGPRINTF ("ignored s @ %p '%s' gc_used#%d", (void*)s, s->u.s.tag, (int) s->gc_used); - } for (s = param_structs; s; s = s->next) if (s->gc_used == GC_POINTED_TO) @@ -2966,22 +2932,21 @@ write_types (outf_p output_header, type_p structur } } else - { - /* Param structure s is not pointed to, so should be ignored. */ DBGPRINTF ("ignored s @ %p", (void*)s); - } if (verbosity_level >= 2) printf ("%s emitted %d routines for %s\n", progname, nbfun, wtd->comment); } -static const struct write_types_data ggc_wtd = { +static const struct write_types_data ggc_wtd = +{ "ggc_m", NULL, "ggc_mark", "ggc_test_and_set_mark", NULL, "GC marker procedures. ", FALSE }; -static const struct write_types_data pch_wtd = { +static const struct write_types_data pch_wtd = +{ "pch_n", "pch_p", "gt_pch_note_object", "gt_pch_note_object", "gt_pch_note_reorder", "PCH type-walking procedures. ", @@ -3068,7 +3033,8 @@ write_local (outf_p output_header, type_p structur return; oprintf (output_header, "\n/* Local pointer-walking routines. */\n"); for (s = structures; s; s = s->next) - if (s->gc_used == GC_POINTED_TO || s->gc_used == GC_MAYBE_POINTED_TO) + if (s->gc_used == GC_POINTED_TO + || s->gc_used == GC_MAYBE_POINTED_TO) { options_p opt; @@ -3080,7 +3046,8 @@ write_local (outf_p output_header, type_p structur { const_type_p const t = (const_type_p) opt->info; if (t->kind == TYPE_STRUCT - || t->kind == TYPE_UNION || t->kind == TYPE_LANG_STRUCT) + || t->kind == TYPE_UNION + || t->kind == TYPE_LANG_STRUCT) { oprintf (output_header, "#define gt_pch_p_"); output_mangled_typename (output_header, s); Please drop. @@ -3254,15 +3226,16 @@ finish_root_table (struct flist *flp, const char * for (fli2 = flp; fli2 && base_files; fli2 = fli2->next) if (fli2->started_p) { - lang_bitmap bitmap = get_lang_bitmap (fli2->name); + lang_bitmap bitmap = get_lang_bitmap (fli2->file); int fnum; for (fnum = 0; bitmap != 0; fnum++, bitmap >>= 1) if (bitmap & 1) { oprintf (base_files[fnum], - "extern const struct %s gt_%s_", tname, pfx); - put_mangled_filename (base_files[fnum], fli2->name); + "extern const struct %s gt_%s_", + tname, pfx); + put_mangled_filename (base_files[fnum], fli2->file); oprintf (base_files[fnum], "[];\n"); } } oprintf (base_files[fnum], "extern const struct %s gt_%s_", tname, pfx); put_mangled_filename (base_files[fnum], fli2->file); @@ -3271,14 +3244,15 @@ finish_root_table (struct flist *flp, const char * size_t fnum; for (fnum = 0; base_files && fnum < num_lang_dirs; fnum++) oprintf (base_files[fnum], - "EXPORTED_CONST struct %s * const %s[] = {\n", tname, name); + "EXPORTED_CONST struct %s * const %s[] = {\n", + tname, name); } @@ -3428,7 +3402,8 @@ write_root (outf_p f, pair_p v, type_p type, const error_at_line (line, "both `%s.%s.%s' and `%s.%s.%s' have tag `%s'", name, fld->name, validf->name, - name, fld->name, ufld->name, tag); + name, fld->name, ufld->name, + tag); validf = ufld; } if (validf != NULL) @@ -3523,7 +3498,9 @@ write_root (outf_p f, pair_p v, type_p type, const break; default: - error_at_line (line, "global `%s' is unimplemented type", name); + error_at_line (line, + "global `%s' is unimplemented type", + name); } } @@ -3549,7 +3526,8 @@ write_array (outf_p f, pair_p v, const struct writ if (wtd->param_prefix) { oprintf (f, "static void gt_%sa_%s\n", wtd->param_prefix, v->name); - oprintf (f, " (void *, void *, gt_pointer_operator, void *);\n"); + oprintf (f, + " (void *, void *, gt_pointer_operator, void *);\n"); oprintf (f, "static void gt_%sa_%s (ATTRIBUTE_UNUSED void *this_obj,\n", wtd->param_prefix, v->name); oprintf (d.of, @@ -3564,7 +3542,8 @@ write_array (outf_p f, pair_p v, const struct writ } d.opt = v->opt; - oprintf (f, "static void gt_%sa_%s (void *);\n", wtd->prefix, v->name); + oprintf (f, "static void gt_%sa_%s (void *);\n", + wtd->prefix, v->name); oprintf (f, "static void\ngt_%sa_%s (ATTRIBUTE_UNUSED void *x_p)\n", wtd->prefix, v->name); oprintf (f, "{\n"); Please drop. @@ -3599,7 +3580,8 @@ write_roots (pair_p variables, bool emit_pch) else if (strcmp (o->name, "param_is") == 0) ; else if (strncmp (o->name, "param", 5) == 0 - && ISDIGIT (o->name[5]) && strcmp (o->name + 6, "_is") == 0) + && ISDIGIT (o->name[5]) + && strcmp (o->name + 6, "_is") == 0) ; else if (strcmp (o->name, "if_marked") == 0) ; Please drop. @@ -3727,8 +3715,7 @@ write_roots (pair_p variables, bool emit_pch) || v->type->u.p->kind != TYPE_PARAM_STRUCT || v->type->u.p->u.param_struct.stru != find_structure ("htab", 0)) { - error_at_line (&v->line, - "if_marked option used but not hash table"); + error_at_line (&v->line, "if_marked option used but not hash table"); continue; } Please drop. @@ -3585,7 +3564,9 @@ write_roots (pair_p variables, bool emit_pch) for (v = variables; v; v = v->next) { - outf_p f = get_output_file_with_visibility (v->line.file); + outf_p f = + get_output_file_with_visibility (CONST_CAST (input_file*, + v->line.file)); Here and everywhere below, break the line before the =, not after: outf_p f = get_output_ .... @@ -3599,7 +3580,8 @@ write_roots (pair_p variables, bool emit_pch) else if (strcmp (o->name, "param_is") == 0) ; else if (strncmp (o->name, "param", 5) == 0 - && ISDIGIT (o->name[5]) && strcmp (o->name + 6, "_is") == 0) + && ISDIGIT (o->name[5]) + && strcmp (o->name + 6, "_is") == 0) ; else if (strcmp (o->name, "if_marked") == 0) ; Please drop this change. @@ -3727,8 +3715,7 @@ write_roots (pair_p variables, bool emit_pch) || v->type->u.p->kind != TYPE_PARAM_STRUCT || v->type->u.p->u.param_struct.stru != find_structure ("htab", 0)) { - error_at_line (&v->line, - "if_marked option used but not hash table"); + error_at_line (&v->line, "if_marked option used but not hash table"); continue; } @@ -3910,10 +3901,8 @@ variable_size_p (const type_p s) return false; } -enum alloc_quantity -{ single, vector }; -enum alloc_zone -{ any_zone, specific_zone }; +enum alloc_quantity { single, vector }; +enum alloc_zone { any_zone, specific_zone }; /* Writes one typed allocator definition for type identifier TYPE_NAME with optional type specifier TYPE_SPECIFIER. The allocator name will contain And this one. @@ -4045,8 +4034,7 @@ output_typename (outf_p of, const_type_p t) { int i; for (i = 0; i < NUM_PARAM; i++) - if (t->u.param_struct.param[i] != NULL) - { + if (t->u.param_struct.param[i] != NULL) { output_typename (of, t->u.param_struct.param[i]); oprintf (of, "_"); } And this one. @@ -4109,32 +4097,15 @@ dump_typekind (int indent, enum typekind kind) printf ("%*ckind = ", indent, ' '); switch (kind) { - case TYPE_SCALAR: - printf ("TYPE_SCALAR"); - break; [...] And all the changes that only reformat code. I have stopped here reading them. + if (*slot) + { + /* Already known input file. */ + free (f); + return (input_file*)(*slot); + } + /* New input file. */ + *slot = f; + return f; + } Watch the indentation of the closing braces. @@ -4577,23 +4594,24 @@ main (int argc, char **argv) if (plugin_output_filename) { size_t ix = 0; - /* In plugin mode, we should have read a state file, and have + /* In plugin mode, we better have read a state file, and have given at least one plugin file. */ if (!read_state_filename) - fatal ("No read state given in plugin mode for %s", - plugin_output_filename); + warning ("No read state given in plugin mode for %s", plugin_output_filename); This is unrelated. Please drop or submit in a separate patch. If you submit a separate patch, correct indentation (two spaces instead of one). > trunk rev 165733. > > ### gcc/ChangeLog entry ### > 2010-10-20 Basile Starynkevitch <basile@starynkevitch.net> > Jeremie Salvucci <basile@starynkevitch.net> > > * gengtype.c (plugin_files, gt_files, this_file, system_file): > Change type to input_file. > (get_file_basename, get_file_realbasename) > (get_file_srcdir_relative_path, get_file_langdir): Change > argument type to input_file. > (error_at_line): Use input_file. > (lang_dir_names, num_lang_dirs): Not static anymore. > (get_lang_bitmap, set_lang_bitmap): Moved to gengtype.h. > (read_input_file_name, read_input_list, note_variable) > (create_field_all, get_file_realbasename) > (get_file_srcdir_relative_path, get_file_basename) > (get_file_langdir, get_file_gtfilename) > (get_output_file_with_visibility, get_output_file_name) > (put_mangled_filename, walk_type, get_output_file_for_structure) > (parse_program_options) > (struct flist): Use input_file. > (input_file_htab): Add variable. > (input_file_by_name, htab_hash_inputfile, htab_eq_inputfile): > Added functions. > (main): Initialize input_file_htab and this_file and > system_file, use input_file. > > * gengtype.h (struct input_file_st, input_file): Add struct and > type. > (CHECK_INPUT_FILE_MAGIC): Added macro. > (gt_files, num_gt_files, this_file, system_file): Moved from > gengtype.c and declared as input_file-s. > (input_file_by_name, get_file_srcdir_relative_path) > (get_lang_bitmap, set_lang_bitmap) > (get_output_file_with_visibility, get_output_file_name): Ditto. > (get_input_file_name): New inlined function. > (struct fileloc): Use input_file. > (lang_dir_names, num_lang_dirs): Moved from gengtype.c. > > * gengtype-lex.l: Include errors.h. > (yybegin): Use input_file_by_name. > > * gengtype-parse.c: Includes errors.h. > (parse_error): Use input_file. > (type): Counts the anonymous pseudo-identifiers. > > * Makefile.in (gengtype-lex.o, gengtype-parse.o): Depends upon > errors.h > > ##### > > Ok for trunk? > > PS. I might not have time to send more patches before the GCC Summit next week > (my slides are unfinished). > > -- > Basile STARYNKEVITCH http://starynkevitch.net/Basile/ > email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359 > 8, rue de la Faiencerie, 92340 Bourg La Reine, France > *** opinions {are only mine, sont seulement les miennes} *** >