Patchwork [pph] Identifier binding fixes. (issue5572065)

login
register
mail settings
Submitter Lawrence Crowl
Date Jan. 26, 2012, 5:09 a.m.
Message ID <20120126050909.95A212225EC@jade.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/137877/
State New
Headers show

Comments

Lawrence Crowl - Jan. 26, 2012, 5:09 a.m.
This change fixes some problems in reconstructing the identifier bindings.
In particular, it removes extra binding creations, handle new bindings better,
identifies already present bindings, and adds some assertions.

Improve debug dump for bindings.

One old test is now passing.  One old test is 2/3 passing.  Two new tests are
passing.  Two old tests are now failing.  These are identical strange failures
with the __sync_fetch_and_add builtin.  The function declaration seems to be
losing its return type.



--
This patch is available for review at http://codereview.appspot.com/5572065

Patch

Index: gcc/testsuite/ChangeLog.pph

	* g++.dg/pph/x0samename3.h: New passing.
	* g++.dg/pph/x1samename.cc: New passing.
	* g++.dg/pph/x4samename.cc: Mark passing.
	* g++.dg/pph/x4resolve1.cc: Change passing assembler diff code.
	* g++.dg/pph/x4resolve2.cc: Change passing assembler diff code.
	* g++.dg/pph/x6dynarray3.cc: Remove two bogus errors.
	* g++.dg/pph/x5dynarray7.h: Add one bogus error.
	* g++.dg/pph/x6dynarray6.h: Add one bogus error.

2012-01-24   Lawrence Crowl  <crowl@google.com>

Index: gcc/cp/ChangeLog.pph

2012-01-25   Lawrence Crowl  <crowl@google.com>

	* pph-core.c (pph_dump_tree_name): Refactor.
	(pph_dump_overload_names): New.
	(pph_dump_one_binding): New.
	(pph_dump_bindings_for_id): New.
	(pph_dump_bindings_for_decl): New.
	(pph_dump_chain): Dump builtins at -fpph-debug>=5.  Remove unneeded
	'next' variable.
	(pph_dump_binding): Likewise.
	(pph_loaded): Clarify purpose of global state dump.
	* name-lookup.c (pph_debug_binding_inaction): Remove redundant output.
	(pph_set_identifier_bindings): Removed.
	(pph_set_chain_identifier_bindings): Removed.
	(pph_set_namespace_bindings): Removed.
	(pph_set_identifier_binding): New.  Refactored from
	pph_set_namespace_bindings.  Fixes some identifer binding issues.
	(pph_set_namespace_decl_binding): New. Refactored from
	pph_set_namespace_bindings.
	(pph_set_chain_namespace_bindings): Use pph_set_namespace_decl_binding
	instead of pph_set_namespace_bindings.
	(pph_set_global_identifier_bindings): Remove bad calls to
	pph_set_chain_identifier_bindings.


Index: gcc/testsuite/g++.dg/pph/x4samename.cc
===================================================================
--- gcc/testsuite/g++.dg/pph/x4samename.cc	(revision 183499)
+++ gcc/testsuite/g++.dg/pph/x4samename.cc	(working copy)
@@ -1,6 +1,3 @@ 
-// { dg-bogus "x4samename.cc:11:18: error: expected unqualified-id before '=' token" "" { xfail *-*-* } 0 }
-// { dg-bogus "x4samename.cc:12:43: error: cannot convert 'const char.' to 'double' for argument '1' to 'int func.double.'" "" { xfail *-*-* } 0 }
-
 #include "x0samename2.h"
 #include "x0samename1.h"
 
Index: gcc/testsuite/g++.dg/pph/x4resolve1.cc
===================================================================
--- gcc/testsuite/g++.dg/pph/x4resolve1.cc	(revision 183499)
+++ gcc/testsuite/g++.dg/pph/x4resolve1.cc	(working copy)
@@ -1,4 +1,4 @@ 
-// pph asm xwant 03374
+// pph asm xwant 53261
 // This test produces overload differences because the declaration and
 // call orders are different between pph and textual parsing.
 
Index: gcc/testsuite/g++.dg/pph/x6dynarray3.cc
===================================================================
--- gcc/testsuite/g++.dg/pph/x6dynarray3.cc	(revision 183499)
+++ gcc/testsuite/g++.dg/pph/x6dynarray3.cc	(working copy)
@@ -1,5 +1,3 @@ 
-// { dg-bogus "a0dynarray-dfn1b.hi:22:5: error: no suitable 'operator delete" "" { xfail *-*-* } 0 }
-// { dg-bogus "a0dynarray-dfn2b.hi:13:9: error: no suitable 'operator delete" "" { xfail *-*-* } 0 }
 // { dg-bogus "a0dynarray-dcl3.hi:11:60: error: call of overloaded 'operator new" "" { xfail *-*-* } 0 }
 
 #include "x5dynarray3.h"
Index: gcc/testsuite/g++.dg/pph/x4resolve2.cc
===================================================================
--- gcc/testsuite/g++.dg/pph/x4resolve2.cc	(revision 183499)
+++ gcc/testsuite/g++.dg/pph/x4resolve2.cc	(working copy)
@@ -1,4 +1,4 @@ 
-// pph asm xwant 37643
+// pph asm xwant 33894
 // This test produces overload differences because the declaration and
 // call orders are different between pph and textual parsing.
 
Index: gcc/testsuite/g++.dg/pph/x6dynarray6.h
===================================================================
--- gcc/testsuite/g++.dg/pph/x6dynarray6.h	(revision 183499)
+++ gcc/testsuite/g++.dg/pph/x6dynarray6.h	(working copy)
@@ -1,3 +1,5 @@ 
+// { dg-bogus "atomicity.h:48:45: error: void value not ignored as it ought to be" "" { xfail *-*-* } 0 }
+
 #ifndef X6DYNARRAY6_H
 #define X6DYNARRAY6_H
 
Index: gcc/testsuite/g++.dg/pph/x5dynarray7.h
===================================================================
--- gcc/testsuite/g++.dg/pph/x5dynarray7.h	(revision 183499)
+++ gcc/testsuite/g++.dg/pph/x5dynarray7.h	(working copy)
@@ -1,3 +1,5 @@ 
+// { dg-bogus "atomicity.h:48:45: error: void value not ignored as it ought to be" "" { xfail *-*-* } 0 }
+
 #ifndef X5DYNARRAY7_H
 #define X5DYNARRAY7_H
 
Index: gcc/cp/pph-core.c
===================================================================
--- gcc/cp/pph-core.c	(revision 183501)
+++ gcc/cp/pph-core.c	(working copy)
@@ -125,27 +125,107 @@  pph_dump_tree_name (FILE *file, tree t, 
 {
   enum tree_code code = TREE_CODE (t);
   const char *text = pph_tree_code_text (code);
-  fprintf (file, "%p ", (void *)t);
+  fprintf (file, "%p %s ", (void *)t, text);
   if (DECL_P (t))
-    fprintf (file, "%s %s\n", text, decl_as_string (t, flags));
+    fprintf (file, "%s\n", decl_as_string (t, flags));
   else if (TYPE_P (t))
-    fprintf (file, "%s %s\n", text, type_as_string (t, flags));
+    fprintf (file, "%s\n", type_as_string (t, flags));
   else if (EXPR_P (t))
-    fprintf (file, "%s %s\n", text, expr_as_string (t, flags));
+    fprintf (file, "%s\n", expr_as_string (t, flags));
   else
     {
-      fprintf (file, "%s ", text );
       print_generic_expr (file, t, flags);
       fprintf (file, "\n");
     }
 }
 
 
+/* Dump a list of overloaded function names to FILE starting with T
+   using FLAGS.  */
+
+static void
+pph_dump_overload_names (FILE *file, tree t, int flags)
+{
+  for (; t; t = OVL_NEXT (t))
+    {
+      tree u = OVL_CURRENT (t);
+      enum tree_code code = TREE_CODE (t);
+      const char *text = pph_tree_code_text (code);
+      fprintf (file, "  binding value: ");
+      fprintf (file, "%p %s ", (void *)t, text);
+      pph_dump_tree_name (file, u, flags);
+    }
+}
+
+
+/* Dump one cxx_binding B to FILE.  */
+
+static void
+pph_dump_one_binding (FILE *file, cxx_binding *b)
+{
+  if (b->scope)
+    {
+      fprintf (file, "  binding scope: ");
+      pph_dump_tree_name (file, b->scope->this_entity, 0);
+    }
+  if (b->value)
+    {
+      if (TREE_CODE (b->value) == OVERLOAD)
+	pph_dump_overload_names (file, b->value, 0);
+      else
+	{
+	  fprintf (file, "  binding value: ");
+	  pph_dump_tree_name (file, b->value, 0);
+	}
+    }
+  if (b->type)
+    {
+      fprintf (file, "  binding type: ");
+      pph_dump_tree_name (file, b->type, 0);
+    }
+  fprintf (file, "  binding is_local: %d\n", b->is_local);
+  fprintf (file, "  binding value_is_inherited: %d\n", b->value_is_inherited);
+}
+
+
+/* Dump all the cxx_bindings for and identifier ID to FILE.  */
+
+static void
+pph_dump_bindings_for_id (FILE *file, tree id)
+{
+  cxx_binding *b = IDENTIFIER_NAMESPACE_BINDINGS (id);
+  if (b)
+    for (; b != NULL; b = b->previous)
+      pph_dump_one_binding (file, b);
+  else
+    fprintf (file, "  binding: NO BINDING\n");
+}
+
+
+/* Dump all the cxx_bindings for and identifier ID to FILE.  */
+
+static void
+pph_dump_bindings_for_decl (FILE *file, tree decl)
+{
+  /* FIXME pph: The declarations often have the same identifier, and
+  hence the same bbindings.  The output is hence redundant.  We should
+  probably just collect the ids an print them in a separate table. */
+
+  tree id = DECL_NAME (decl);
+  if (id)
+    pph_dump_bindings_for_id (file, id);
+  else
+    fprintf (file, "  binding: NO NAME\n" );
+}
+
+
+/* Forward declaration.  */
+
 static void
 pph_dump_binding (FILE *file, cp_binding_level *level);
 
 
-/* Dump namespace NS for PPH.  */
+/* Dump namespace NS to FILE.  */
 
 static void
 pph_dump_namespace (FILE *file, tree ns)
@@ -163,12 +243,14 @@  pph_dump_namespace (FILE *file, tree ns)
 static void
 pph_dump_chain (FILE *file, tree chain)
 {
-  tree t, next;
-  for (t = chain; t; t = next)
+  tree t;
+  for (t = chain; t; t = DECL_CHAIN (t))
     {
-      next = DECL_CHAIN (t);
-      if (!DECL_IS_BUILTIN (t))
-        pph_dump_tree_name (file, t, 0);
+      if (!DECL_IS_BUILTIN (t) || flag_pph_debug >= 5)
+	{
+	  pph_dump_tree_name (file, t, 0);
+	  pph_dump_bindings_for_decl (file, t);
+	}
     }
 }
 
@@ -178,12 +260,11 @@  pph_dump_chain (FILE *file, tree chain)
 void
 pph_dump_binding (FILE *file, cp_binding_level *level)
 {
-  tree t, next;
+  tree t;
   pph_dump_chain (file, level->names);
-  for (t = level->namespaces; t; t = next)
+  for (t = level->namespaces; t; t = DECL_CHAIN (t))
     {
-      next = DECL_CHAIN (t);
-      if (!DECL_IS_BUILTIN (t))
+      if (!DECL_IS_BUILTIN (t) || flag_pph_debug >= 5)
         pph_dump_namespace (file, t);
     }
 }
@@ -900,7 +981,8 @@  pph_loaded (void)
 {
   pph_set_global_identifier_bindings();
   if (flag_pph_dump_tree)
-    pph_dump_global_state (pph_logfile, "after all pph read");
+    /* FIXME pph: We could probably just dump the identifier bindings.  */
+    pph_dump_global_state (pph_logfile, "after identifiers bound");
 }
 
 
Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(revision 183501)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -6131,7 +6131,7 @@  pph_debug_binding_inaction (const char *
 {
   if (flag_pph_debug >= 5 || (flag_pph_debug >= 3 && !DECL_IS_BUILTIN (decl)))
     {
-      fprintf (pph_logfile, "PPH: not %s ", action);
+      fprintf (pph_logfile, "PPH: %s ", action);
       pph_dump_tree_name (pph_logfile, decl, 0);
     }
 }
@@ -6153,98 +6153,98 @@  pph_foreach_on_chain_bl (tree first, cp_
 }
 
 
-/* Set a identifier binding.  */
+/*  Set an identifier binding.  */
 
 static void
-pph_set_identifier_bindings (tree decl, cp_binding_level *bl,
-			     int flags ATTRIBUTE_UNUSED)
+pph_set_identifier_binding (tree id, tree decl,
+			    cp_binding_level *bl, int flags)
 {
-  /* Set the identifier binding for a single decl.  */
-  tree id = DECL_NAME (decl);
-  if (id)
+  tree old_value;
+
+  /* FIXME pph: This code plagarizes from push_overloaded_decl_1 and
+     binding_for_name.  It may be incomplete.  */
+
+  cxx_binding *b = cp_binding_level_find_binding_for_name (bl, id);
+  if (!b)
     {
-      pph_debug_binding_action ("push bind", decl);
-      push_binding (id, decl, bl);
+      b = cxx_binding_make_for_name (bl, id);
+      b->value = decl;
+      pph_debug_binding_action ("new bind", decl);
+      return;
     }
-}
 
+  old_value = b->value;
+  gcc_assert (old_value != NULL);
+  if (is_overloaded_fn (old_value))
+    {
+      /* We don't overload implicit built-ins.  duplicate_decls()
+    	 may fail to merge the decls if the new decl is e.g. a
+    	 template function.  */
+      if (TREE_CODE (old_value) == FUNCTION_DECL
+    	  && DECL_ANTICIPATED (old_value)
+    	  && !DECL_HIDDEN_FRIEND_P (old_value))
+    	old_value = NULL;
+      decl = modify_binding_for_overload (decl, old_value, flags);
+      if (TREE_CODE (decl) == OVERLOAD)
+        pph_debug_overload_binding_action ("mod bind", decl);
+      else
+        pph_debug_binding_action ("mod bind", decl);
+    }
 
-/* Set the identifier bindings for an individual chain.  */
+  /* FIXME pph: This code plagarizes from set_namespace_binding.
+     It has trouble with supplement_binding, methinks.  */
 
-static void
-pph_set_chain_identifier_bindings (tree first, cp_binding_level *bl, int flags)
-{
-  pph_foreach_on_chain_bl (first, bl, flags, pph_set_identifier_bindings);
+  if (!old_value)
+    {
+      /* The old value must have been set to null above.  */
+      b->value = decl;
+      pph_debug_binding_action ("rwr bind", decl);
+    }
+  else if (b->value == decl)
+    {
+      pph_debug_binding_action ("same bind", decl);
+    }
+  else if (TREE_CODE (decl) == OVERLOAD)
+    {
+      b->value = decl;
+      pph_debug_overload_binding_action ("ovl bind", decl);
+    }
+  else if (TREE_CODE (b->value) == TYPE_DECL &&
+       TREE_CODE (decl) != TYPE_DECL)
+    {
+      b->type = b->value;
+      b->value = decl;
+      pph_debug_binding_action ("t/v bind", decl);
+    }
+  else if (TREE_CODE (b->value) != TYPE_DECL &&
+       TREE_CODE (decl) == TYPE_DECL)
+    {
+      b->type = decl;
+      pph_debug_binding_action ("v/t bind", decl);
+    }
+  else
+    pph_debug_binding_inaction ("not bind", decl);
 }
 
 
-/*  Set a namespace identifier binding.  */
+/*  Set a namespace declaration binding.  */
 
 static void
-pph_set_namespace_bindings (tree decl, cp_binding_level *bl, int flags)
+pph_set_namespace_decl_binding (tree decl, cp_binding_level *bl, int flags)
 {
   /* Set the namespace identifier binding for a single decl.  */
   tree id = DECL_NAME (decl);
   if (id)
-    {
-      /* FIXME pph: This code plagarizes from push_overloaded_decl_1 and
-	 binding_for_name.  It may be incomplete.  */
-      cxx_binding *b = cp_binding_level_find_binding_for_name (bl, id);
-      if (b)
-	{
-	  tree old = b->value;
-	  if (is_overloaded_fn (old))
-	    {
-	      /* We don't overload implicit built-ins.  duplicate_decls()
-		 may fail to merge the decls if the new decl is e.g. a
-		 template function.  */
-              if (TREE_CODE (old) == FUNCTION_DECL
-		  && DECL_ANTICIPATED (old)
-		  && !DECL_HIDDEN_FRIEND_P (old))
-		old = NULL;
-              decl = modify_binding_for_overload (decl, old, flags);
-	    }
-	}
-      else
-	b = cxx_binding_make_for_name (bl, id);
-
-      /* FIXME pph: This code plagarizes from set_namespace_binding.
-	 It has trouble with supplement_binding, methinks.  */
-      if (!b->value)
-	{
-	  b->value = decl;
-	  pph_debug_binding_action ("new bind", decl);
-	}
-      if (TREE_CODE (decl) == OVERLOAD)
-	{
-	  b->value = decl;
-	  pph_debug_overload_binding_action ("ovl bind", decl);
-	}
-      else if (TREE_CODE (b->value) == TYPE_DECL &&
-    	   TREE_CODE (decl) != TYPE_DECL)
-	{
-	  b->type = b->value;
-	  b->value = decl;
-	  pph_debug_binding_action ("t/v bind", decl);
-	}
-      else if (TREE_CODE (b->value) != TYPE_DECL &&
-    	   TREE_CODE (decl) == TYPE_DECL)
-	{
-	  b->type = decl;
-	  pph_debug_binding_action ("v/t bind", decl);
-	}
-      else
-	pph_debug_binding_inaction ("not bind", decl);
-    }
+    pph_set_identifier_binding (id, decl, bl, flags);
 }
 
 
-/*  Set the namespace identifier bindings.  */
+/*  Set the namespace declaration bindings.  */
 
 static void
 pph_set_chain_namespace_bindings (tree first, cp_binding_level *bl, int flags)
 {
-  pph_foreach_on_chain_bl (first, bl, flags, pph_set_namespace_bindings);
+  pph_foreach_on_chain_bl (first, bl, flags, pph_set_namespace_decl_binding);
 }
 
 
@@ -6303,10 +6303,6 @@  void
 pph_set_global_identifier_bindings (void)
 {
   cp_binding_level *bl = scope_chain->bindings;
-  pph_set_chain_identifier_bindings (bl->names, bl, 0);
-  pph_set_chain_identifier_bindings (bl->namespaces, bl, 0);
-  pph_set_chain_identifier_bindings (bl->usings, bl, PUSH_USING);
-  pph_set_chain_identifier_bindings (bl->using_directives, bl, 0);
   pph_set_namespace_namespace_bindings (bl);
 }