This patch addresses two main problems. (1) We were mishandling tagless types. The root of the problem was that tagless types were given tags based on a sequence number. Two different PPH files would thus have the same manufactured tag name for different types, leading to inappropriate collisions in lookup. The solution is to manufacture the tag from the source location rather than from a sequence number. All of this construction must match the pattern when the name is later strcmp'ed to see if it is actually tagless.
Tests c1anonymous1.h and c1anonymous2.h are now passing. Added tests p0anonretag.h and p1anonretag.cc to ensure that the pattern matcher works. (2) We were getting macro redefinition errors when two PPH files textually included the same #defines because each PPH file replays its definitions. The applied fix is to not do idempotent defines. Test c7features.cc is now passing. Test x7rtti.cc does not have as many errors. This patch also addresses a minor problem. (3) Debugging test x0tmpldfltparm.h was difficult because the same template parameter identifier was used in different places. Instead, use unique names. (4) Some debug/dump/print routines were missing null pointer checks. Add them. Tested on x64. Index: gcc/testsuite/ChangeLog.pph 2012-03-16 Lawrence Crowl <cr...@google.com> * g++.dg/pph/README: Clarify p category. * g++.dg/pph/c1anonymous1.h: Mark passing. * g++.dg/pph/c1anonymous2.h: Mark passing. * g++.dg/pph/c7features.cc: Mark passing. * g++.dg/pph/p0anonretag.h: New. * g++.dg/pph/p1anonretag.cc: New. * g++.dg/pph/x0tmpldfltparm.h: Disambiguate template param names. * g++.dg/pph/x7rtti.cc: Remove some xfails. Index: gcc/cp/ChangeLog.pph 2012-03-16 Lawrence Crowl <cr...@google.com> * cp-tree.h (make_anon_name): Add location parameter. * name-lookup.c (static const char anon_char): New. (static const char anon_prefix): New. (edit_anon_name): New. (make_anon_name): Add location parameter. Use location in creating names for anonymous (tagless) types when using PPH. (make_lambda_name): Fix comment for later. * class.c (layout_class_type): Pass location to make_anon_name. * decl.c (start_enum): Pass location to make_anon_name. * semantics.c (finish_compound_literal): Pass location to make_anon_name. (begin_class_definition): Pass location to make_anon_name. * error.c (dump_type): Protect call against null pointer. * parser.c (cp_lexer_print_token): Protect against null values. (cp_parser_enum_specifier): Pass location to make_anon_name. (cp_parser_class_head): Pass location to make_anon_name. Index: libcpp/ChangeLog.pph 2012-03-16 Lawrence Crowl <cr...@google.com> * symtab.c (cpp_lt_already_done): New. (cpp_lt_replay): Avoid idempotent macro defines. Index: gcc/testsuite/g++.dg/pph/c1anonymous1.h =================================================================== --- gcc/testsuite/g++.dg/pph/c1anonymous1.h (revision 185481) +++ gcc/testsuite/g++.dg/pph/c1anonymous1.h (working copy) @@ -1,11 +1,8 @@ -// { xfail-if "ANONYMOUS MERGING" { "*-*-*" } { "-fpph-map=pph.map" } } -// { dg-bogus "c0anonymous.h:4:16: error: 'anon_t' has a previous declaration here" "" { xfail *-*-* } 0 } - #ifndef C1ANONYMOUS #define C1ANONYMOUS #include "c0anonymous.h" -enum { first, second }; // { dg-bogus "'anon_t' referred to as enum" "" { xfail *-*-* } } +enum { first, second }; #endif Index: gcc/testsuite/g++.dg/pph/c1anonymous2.h =================================================================== --- gcc/testsuite/g++.dg/pph/c1anonymous2.h (revision 185481) +++ gcc/testsuite/g++.dg/pph/c1anonymous2.h (working copy) @@ -1,14 +1,11 @@ -// { xfail-if "ANONYMOUS MERGING" { "*-*-*" } { "-fpph-map=pph.map" } } -// { dg-bogus "c0anonymous.h:4:16: error: 'struct anon_t' has a previous declaration as 'struct anon_t'" "" { xfail *-*-* } 0 } - #ifndef C1ANONYMOUS2_H #define C1ANONYMOUS2_H #include "c0anonymous.h" -typedef struct { // { dg-bogus "conflicting declaration 'struct<anonymous>'" "" { xfail *-*-* } } +typedef struct { char *field; -} anon2_t; // { dg-bogus "invalid type in declaration before ';' token" "" { xfail *-*-* } } +} anon2_t; #endif Index: gcc/testsuite/g++.dg/pph/README =================================================================== --- gcc/testsuite/g++.dg/pph/README (revision 185481) +++ gcc/testsuite/g++.dg/pph/README (working copy) @@ -7,9 +7,10 @@ names. c - positive tests for C-level headers and sources d - negative tests for C-level headers and sources e - C-level tests for non-sharable headers - p - positive tests for what would be c-level code, but which - due to C++ standard namespace games are not quite C level - tests + p - positive tests for what would be c-level code, but which due to + C++ standard namespace games and + implicit tagging from typedefs + are not quite C level tests x - C++-level positive tests y - C++-level negative tests z - C++-level tests for non-sharable headers Index: gcc/testsuite/g++.dg/pph/x0tmpldfltparm.h =================================================================== --- gcc/testsuite/g++.dg/pph/x0tmpldfltparm.h (revision 185481) +++ gcc/testsuite/g++.dg/pph/x0tmpldfltparm.h (working copy) @@ -1,12 +1,12 @@ #ifndef X0TMPLDFLTPARM_H #define X0TMPLDFLTPARM_H -template< typename T > +template< typename A > struct auxillary { }; -template< typename T, typename U = auxillary< T > > +template< typename P, typename Q = auxillary< P > > struct primary { }; Index: gcc/testsuite/g++.dg/pph/x7rtti.cc =================================================================== --- gcc/testsuite/g++.dg/pph/x7rtti.cc (revision 185481) +++ gcc/testsuite/g++.dg/pph/x7rtti.cc (working copy) @@ -1,8 +1,5 @@ // FIXME pph: This should be a { dg=do run } (with '=' replaced by '-') -// { xfail-if "UNKNOWN MACRO AND BOGUS RTTI" { "*-*-*" } { "-fpph-map=pph.map" } } -// { dg-bogus "warning: .__STDC_IEC_559_COMPLEX__. redefined" "" { xfail *-*-* } 0 } -// { dg-bogus "warning: .__STDC_ISO_10646__. redefined" "" { xfail *-*-* } 0 } -// { dg-bogus "warning: .__STDC_IEC_559__. redefined" "" { xfail *-*-* } 0 } +// { xfail-if "BOGUS RTTI" { "*-*-*" } { "-fpph-map=pph.map" } } #include "x5rtti1.h" #include "x5rtti2.h" Index: gcc/testsuite/g++.dg/pph/c7features.cc =================================================================== --- gcc/testsuite/g++.dg/pph/c7features.cc (revision 185481) +++ gcc/testsuite/g++.dg/pph/c7features.cc (working copy) @@ -1,7 +1,2 @@ -// { xfail-if "UNKNOWN" { "*-*-*" } { "-fpph-map=pph.map" } } -// { dg-bogus "warning: .__STDC_IEC_559_COMPLEX__. redefined" "" { xfail *-*-* } 0 } -// { dg-bogus "warning: .__STDC_ISO_10646__. redefined" "" { xfail *-*-* } 0 } -// { dg-bogus "warning: .__STDC_IEC_559__. redefined" "" { xfail *-*-* } 0 } - #include "c5features1.h" #include "c5features2.h" Index: gcc/testsuite/g++.dg/pph/p1anonretag.cc =================================================================== --- gcc/testsuite/g++.dg/pph/p1anonretag.cc (revision 0) +++ gcc/testsuite/g++.dg/pph/p1anonretag.cc (revision 0) @@ -0,0 +1,11 @@ +#include "p0anonretag.h" + +int F1 (S1 *p) +{ + return p != 0; +} + +int F2 (S2 *p) +{ + return p != 0; +} Index: gcc/testsuite/g++.dg/pph/p0anonretag.h =================================================================== --- gcc/testsuite/g++.dg/pph/p0anonretag.h (revision 0) +++ gcc/testsuite/g++.dg/pph/p0anonretag.h (revision 0) @@ -0,0 +1,19 @@ +#ifndef P0ANONRETAG_H +#define P0ANONRETAG_H + +typedef struct S1 +{ + unsigned long s1; + struct S1 *s2; + char *s3; +} S1; + +typedef struct +{ + unsigned int s4; + unsigned int s5; + int s6; + unsigned int *s7; +} S2; + +#endif Index: gcc/cp/class.c =================================================================== --- gcc/cp/class.c (revision 185481) +++ gcc/cp/class.c (working copy) @@ -5640,8 +5640,9 @@ layout_class_type (tree t, tree *virtual temporarily give the field a name. */ if (PCC_BITFIELD_TYPE_MATTERS && !DECL_NAME (field)) { + location_t loc = DECL_SOURCE_LOCATION (field); was_unnamed_p = true; - DECL_NAME (field) = make_anon_name (); + DECL_NAME (field) = make_anon_name (loc); } #endif DECL_SIZE (field) = TYPE_SIZE (integer_type); Index: gcc/cp/decl.c =================================================================== --- gcc/cp/decl.c (revision 185481) +++ gcc/cp/decl.c (working copy) @@ -12093,7 +12093,8 @@ start_enum (tree name, tree enumtype, tr continue. */ if (enumtype == error_mark_node) { - name = make_anon_name (); + location_t loc = DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)); + name = make_anon_name (loc); enumtype = NULL_TREE; } Index: gcc/cp/error.c =================================================================== --- gcc/cp/error.c (revision 185481) +++ gcc/cp/error.c (working copy) @@ -485,10 +485,10 @@ dump_type (tree t, int flags) { tree decl = TYPE_NAME (t); pp_cxx_cv_qualifier_seq (cxx_pp, t); - dump_location_qualifier (decl, flags); if (decl) { tree ident = DECL_NAME (decl); + dump_location_qualifier (decl, flags); if (ident) pp_cxx_tree_identifier (cxx_pp, ident); else Index: gcc/cp/semantics.c =================================================================== --- gcc/cp/semantics.c (revision 185481) +++ gcc/cp/semantics.c (working copy) @@ -2416,7 +2416,7 @@ finish_compound_literal (tree type, tree } cp_apply_type_quals_to_decl (cp_type_quals (type), decl); decl = pushdecl_top_level (decl); - DECL_NAME (decl) = make_anon_name (); + DECL_NAME (decl) = make_anon_name (DECL_SOURCE_LOCATION (decl)); SET_DECL_ASSEMBLER_NAME (decl, DECL_NAME (decl)); return decl; } @@ -2561,7 +2561,7 @@ begin_class_definition (tree t, tree att if (t == error_mark_node || ! MAYBE_CLASS_TYPE_P (t)) { t = make_class_type (RECORD_TYPE); - pushtag (make_anon_name (), t, /*tag_scope=*/ts_current); + pushtag (make_anon_name (input_location), t, /*tag_scope=*/ts_current); } if (TYPE_BEING_DEFINED (t)) Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 185481) +++ gcc/cp/parser.c (working copy) @@ -1193,11 +1193,22 @@ cp_lexer_print_token (FILE * stream, cp_ case CPP_KEYWORD: /* Some keywords have a value that is not an IDENTIFIER_NODE. For example, `struct' is mapped to an INTEGER_CST. */ - if (TREE_CODE (token->u.value) != IDENTIFIER_NODE) - break; - /* else fall through */ + if (token->u.value) + { + if (TREE_CODE (token->u.value) != IDENTIFIER_NODE) + break; + /* else fall through */ + } + else + { + fputs ("<nil>", stream); + break; + } case CPP_NAME: - fputs (IDENTIFIER_POINTER (token->u.value), stream); + if (token->u.value) + fputs (IDENTIFIER_POINTER (token->u.value), stream); + else + fputs ("<nil>", stream); break; case CPP_STRING: @@ -14446,7 +14457,7 @@ cp_parser_enum_specifier (cp_parser* par identifier = cp_parser_identifier (parser); else { - identifier = make_anon_name (); + identifier = make_anon_name (input_location); is_anonymous = true; } } @@ -18636,7 +18647,7 @@ cp_parser_class_head (cp_parser* parser, { /* If the class was unnamed, create a dummy name. */ if (!id) - id = make_anon_name (); + id = make_anon_name (input_location); type = xref_tag (class_key, id, /*tag_scope=*/ts_current, parser->num_template_parameter_lists); } Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h (revision 185481) +++ gcc/cp/cp-tree.h (working copy) @@ -5000,7 +5000,7 @@ extern tree pushdecl (tree); extern tree pushdecl_maybe_friend (tree, bool); extern void maybe_push_cleanup_level (tree); extern tree pushtag (tree, tree, tag_scope); -extern tree make_anon_name (void); +extern tree make_anon_name (location_t); extern tree pushdecl_top_level_maybe_friend (tree, bool); extern tree pushdecl_top_level_and_finish (tree, tree); extern tree check_for_out_of_scope_variable (tree); Index: gcc/cp/name-lookup.c =================================================================== --- gcc/cp/name-lookup.c (revision 185481) +++ gcc/cp/name-lookup.c (working copy) @@ -1978,15 +1978,54 @@ constructor_name_p (tree name, tree type static GTY(()) int anon_cnt; +/* Anonymous names may allow a character outside the C++ set. */ + +# ifdef JOINER +static const char anon_char = JOINER; +static const char anon_prefix[] = { JOINER, '_' }; +# else +static const char anon_char = '_'; +static const char anon_prefix = ANON_AGGRNAME_PREFIX; +# endif + +/* Edit a BUFFER so that every character + is within the assembler identifier set. */ + +static void +edit_anon_name (char *buffer) +{ + char value; + for (value = *buffer; value != '\0'; value = *++buffer) + { + if ( ('0' <= value && value <= '9') + || ('A' <= value && value <= 'Z') + || ('a' <= value && value <= 'z') + || (value == '_')) + { + /* The character is already within the identifier set. */ + continue; + } + *buffer = anon_char; /* Change bad character to good one. */ + } +} + /* Return an IDENTIFIER which can be used as a name for anonymous structs and unions. */ tree -make_anon_name (void) +make_anon_name (location_t loc) { - char buf[32]; + char buf[MAXPATHLEN + 64]; - sprintf (buf, ANON_AGGRNAME_FORMAT, anon_cnt++); + if (pph_enabled_p ()) + { + expanded_location xloc = expand_location (loc); + sprintf (buf, "%s%s:%d:%d", + anon_prefix, xloc.file, xloc.line, xloc.column); + edit_anon_name (buf); + } + else + sprintf (buf, ANON_AGGRNAME_FORMAT, anon_cnt++); return get_identifier (buf); } @@ -2003,6 +2042,7 @@ make_lambda_name (void) { char buf[32]; + /* FIXME pph: We will want to tie this name to location for PPH. */ sprintf (buf, LAMBDANAME_FORMAT, lambda_cnt++); return get_identifier (buf); } Index: libcpp/symtab.c =================================================================== --- libcpp/symtab.c (revision 185481) +++ libcpp/symtab.c (working copy) @@ -817,6 +817,30 @@ cpp_lt_define_syntax (char *needed, cons *needed++ = '\0'; } + +/* Return true if a replay in the READER for a definition of the macro IDENT + to a WANTED value has already been done. */ + +static bool +cpp_lt_define_done (cpp_reader *reader, + const char *ident_str, unsigned int ident_len, + const char *wanted_str) +{ + hashnode node; + cpp_hashnode *cpp_node; + const char *existing_str; + const unsigned char *ident_alt = (const unsigned char *)ident_str; + node = ht_lookup (reader->hash_table, ident_alt, ident_len, HT_NO_INSERT); + if (!node) + return false; + cpp_node = CPP_HASHNODE (node); + if (cpp_node->type != NT_MACRO) + return false; + existing_str = lt_query_macro (reader, cpp_node); + return strcmp (wanted_str, existing_str) == 0; +} + + /* Replay the macro definitions captured by the table of IDENTIFIERS into the READER state. If LOC is non-null, assign *LOC as the source_location to all macro definitions replayed. */ @@ -855,8 +879,12 @@ cpp_lt_replay (cpp_reader *reader, cpp_i { if (after_str != NULL) { - cpp_lt_define_syntax (buffer, ident_str, after_str); - cpp_define (reader, buffer); + if (!cpp_lt_define_done (reader, ident_str, entry->ident_len, + after_str)) + { + cpp_lt_define_syntax (buffer, ident_str, after_str); + cpp_define (reader, buffer); + } } /* else consistently not macros */ } @@ -868,9 +896,13 @@ cpp_lt_replay (cpp_reader *reader, cpp_i } else if (strcmp (before_str, after_str) != 0) { - cpp_undef (reader, ident_str); - cpp_lt_define_syntax (buffer, ident_str, after_str); - cpp_define (reader, buffer); + if (!cpp_lt_define_done (reader, ident_str, entry->ident_len, + after_str)) + { + cpp_undef (reader, ident_str); + cpp_lt_define_syntax (buffer, ident_str, after_str); + cpp_define (reader, buffer); + } } /* else macro with the same definition */ } -- This patch is available for review at http://codereview.appspot.com/5846054