c++: ICE with using-decl [PR 99238]

This ICE was caused by a stray TREE_VISITED marker.  The lookup
machinery was leaving it there due to the way I'd arranged for it to
be cleared.  That was presuming the name_lookup::value field didn't
change, and that wasn't always true in the using-decl processing.  I
took the opportunity to break out a helper, and then call it
immediately after lookups, rather than wait until destructor time.
Added some asserts the module machinery to catch further cases of
this.

	PR c++/99238
	gcc/cp/
	* module.cc (depset:#️⃣:add_binding_entity): Assert not
	visited.
	(depset::add::add_specializations): Likewise.
	* name-lookup.c (name_lookup::dedup): New.
	(name_lookup::~name_lookup): Assert not deduping.
	(name_lookup::restore_state): Likewise.
	(name_lookup::add_overload): Replace outlined code with dedup
	call.
	(name_lookup::add_value): Likewise.
	(name_lookup::search_namespace_only): Likewise.
	(name_lookup::adl_namespace_fns): Likewise.
	(name_lookup::adl_class_fns): Likewise.
	(name_lookup::search_adl): Likewise.  Add clearing dedup call.
	(name_lookup::search_qualified): Likewise.
	(name_lookup::search_unqualified): Likewise.
	gcc/testsuite/
	* g++.dg/modules/pr99238.h: New.
	* g++.dg/modules/pr99238_a.H: New.
	* g++.dg/modules/pr99238_b.H: New.

gcc/cp/module.cc

@@ -12706,6 +12706,9 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
 	  *slot = data->binding;
 	}
 
+      /* Make sure nobody left a tree visited lying about.  */
+      gcc_checking_assert (!TREE_VISITED (decl));
+
       if (flags & WMB_Using)
 	{
 	  decl = ovl_make (decl, NULL_TREE);
@@ -13000,6 +13003,8 @@ depset::hash::add_specializations (bool decl_p)
     have_spec:;
 #endif
 
+      /* Make sure nobody left a tree visited lying about.  */
+      gcc_checking_assert (!TREE_VISITED (spec));
       depset *dep = make_dependency (spec, depset::EK_SPECIALIZATION);
       if (dep->is_special ())
 	{

gcc/cp/name-lookup.c

@@ -464,6 +464,7 @@ public:
   }
   ~name_lookup ()
   {
+    gcc_checking_assert (!deduping);
     restore_state ();
   }
 
@@ -471,6 +472,17 @@ private: /* Uncopyable, unmovable, unassignable. I am a rock. */
   name_lookup (const name_lookup &);
   name_lookup &operator= (const name_lookup &);
 
+ public:
+  /* Turn on or off deduping mode.  */
+  void dedup (bool state)
+  {
+    if (deduping != state)
+      {
+	deduping = state;
+	lookup_mark (value, state);
+      }
+  }
+
 protected:
   static bool seen_p (tree scope)
   {
@@ -605,8 +617,7 @@ name_lookup::preserve_state ()
 void
 name_lookup::restore_state ()
 {
-  if (deduping)
+  gcc_checking_assert (!deduping);
-    lookup_mark (value, false);
 
   /* Unmark and empty this lookup's scope stack.  */
   for (unsigned ix = vec_safe_length (scopes); ix--;)
@@ -703,12 +714,9 @@ name_lookup::add_overload (tree fns)
 	probe = ovl_skip_hidden (probe);
       if (probe && TREE_CODE (probe) == OVERLOAD
 	  && OVL_DEDUP_P (probe))
-	{
+	/* We're about to add something found by multiple paths, so need to
-	  /* We're about to add something found by multiple paths, so
+	   engage deduping mode.  */
-	     need to engage deduping mode.  */
+	dedup (true);
-	  lookup_mark (value, true);
-	  deduping = true;
-	}
     }
 
   value = lookup_maybe_add (fns, value, deduping);
@@ -737,12 +745,8 @@ name_lookup::add_value (tree new_val)
     value = ORIGINAL_NAMESPACE (value);
   else
     {
-      if (deduping)
+      /* Disengage deduping mode.  */
-	{
+      dedup (false);
-	  /* Disengage deduping mode.  */
-	  lookup_mark (value, false);
-	  deduping = false;
-	}
       value = ambiguous (new_val, value);
     }
 }
@@ -951,10 +955,7 @@ name_lookup::search_namespace_only (tree scope)
 			    if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
 				|| (hit & 2
 				    && BINDING_VECTOR_PARTITION_DUPS_P (val)))
-			      {
+			      dedup (true);
-				lookup_mark (value, true);
-				deduping = true;
-			      }
 			  }
 			dup_detect |= dup;
 		      }
@@ -1076,6 +1077,8 @@ name_lookup::search_qualified (tree scope, bool usings)
 	found = search_usings (scope);
     }
 
+  dedup (false);
+
   return found;
 }
 
@@ -1177,6 +1180,8 @@ name_lookup::search_unqualified (tree scope, cp_binding_level *level)
 	break;
     }
 
+  dedup (false);
+
   /* Restore to incoming length.  */
   vec_safe_truncate (queue, length);
 
@@ -1284,15 +1289,10 @@ name_lookup::adl_namespace_fns (tree scope, bitmap imports)
 			else if (MODULE_BINDING_PARTITION_P (bind))
 			  dup = 2;
 			if (unsigned hit = dup_detect & dup)
-			  {
+			  if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
-			    if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
+			      || (hit & 2
-				|| (hit & 2
+				  && BINDING_VECTOR_PARTITION_DUPS_P (val)))
-				    && BINDING_VECTOR_PARTITION_DUPS_P (val)))
+			    dedup (true);
-			      {
-				lookup_mark (value, true);
-				deduping = true;
-			      }
-			  }
 			dup_detect |= dup;
 		      }
 
@@ -1328,11 +1328,7 @@ name_lookup::adl_class_fns (tree type)
 	    if (CP_DECL_CONTEXT (fn) != context)
 	      continue;
 
-	    if (!deduping)
+	    dedup (true);
-	      {
-		lookup_mark (value, true);
-		deduping = true;
-	      }
 
 	    /* Template specializations are never found by name lookup.
 	       (Templates themselves can be found, but not template
@@ -1634,12 +1630,9 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc> *args)
   if (vec_safe_length (scopes))
     {
       /* Now do the lookups.  */
-      if (fns)
-	{
-	  deduping = true;
-	  lookup_mark (fns, true);
-	}
       value = fns;
+      if (fns)
+	dedup (true);
 
       /* INST_PATH will be NULL, if this is /not/ 2nd-phase ADL.  */
       bitmap inst_path = NULL;
@@ -1697,14 +1690,9 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc> *args)
 
 		    if (tree bind = *mslot)
 		      {
-			if (!deduping)
+			/* We must turn on deduping, because some other class
-			  {
+			   from this module might also be in this namespace.  */
-			    /* We must turn on deduping, because some
+			dedup (true);
-			       other class from this module might also
-			       be in this namespace.  */
-			    deduping = true;
-			    lookup_mark (value, true);
-			  }
 
 			/* Add the exported fns  */
 			if (STAT_HACK_P (bind))
@@ -1715,6 +1703,7 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc> *args)
 	}
 
       fns = value;
+      dedup (false);
     }
 
   return fns;

gcc/testsuite/g++.dg/modules/pr99238.h

@@ -0,0 +1 @@
+struct tm;

gcc/testsuite/g++.dg/modules/pr99238_a.H

@@ -0,0 +1,4 @@
+// PR 99238 ICE with using decl
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+#include "pr99238.h"

gcc/testsuite/g++.dg/modules/pr99238_b.H

@@ -0,0 +1,8 @@
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+#include "pr99238.h"
+import "pr99238_a.H";
+namespace std
+{
+  using ::tm;
+}