c++/modules: Support ADL on non-discarded GM entities [PR121705]

[basic.lookup.argdep] p4 says that ADL also finds declarations of
functions or function templates from a point of lookup within the
module, only ignoring discarded (or internal) GM entities.

To implement this we need to create bindings for these entities so that
we can guarantee that name lookup will discover they exist.  This raises
some complications, though, as we ideally would like to avoid having
bindings that contain no declarations, or emitting GM namespaces that
only contain discarded or internal functions.

This patch does this by additionally creating a new binding whenever we
call make_dependency on a non-EK_FOR_BINDING decl.  We don't do this for
using-decls, as at the point of use of a GM entity we no longer know
whether we called through a using-decl or the declaration directly;
however, this behaviour is explicitly supported by [module.global.frag]
p3.6.

Creating these bindings caused g++.dg/modules/default-arg-4_* to fail.
It turns out that this makes the behaviour look identical to
g++.dg/modules/default-arg-5, which is incorrectly dg-error-ing default
value redeclarations (we only currently error because of PR c++/99000).
This patch removes the otherwise identical test and turns the dg-errors
into xfailed dg-bogus.

As a drive-by fix this also fixes an ICE when debug printing friend
function instantiations.

	PR c++/121705
	PR c++/117658

gcc/cp/ChangeLog:

	* module.cc (depset::hash::make_dependency): Make bindings for
	GM functions.
	(depset::hash::add_binding_entity): Adjust comment.
	(depset::hash::add_deduction_guides): Add log.
	* ptree.cc (cxx_print_xnode): Handle friend functions where
	TI_TEMPLATE is an OVERLOAD or IDENTIFIER.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/default-arg-4_a.C: XFAIL bogus errors.
	* g++.dg/modules/default-arg-4_b.C: Likewise.
	* g++.dg/modules/default-arg-5_a.C: Remove duplicate test.
	* g++.dg/modules/default-arg-5_b.C: Likewise.
	* g++.dg/modules/adl-9_a.C: New test.
	* g++.dg/modules/adl-9_b.C: New test.
	* g++.dg/modules/gmf-5.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>

9 files changed, 131 insertions, 63 deletions

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ee76dd8..170a0bd 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -14314,6 +14314,54 @@ depset::hash::make_dependency (tree decl, entity_kind ek)
 	      /* Anonymous types can't be forward-declared.  */
 	      && !IDENTIFIER_ANON_P (DECL_NAME (not_tmpl)))
 	    dep->set_flag_bit<DB_IS_PENDING_BIT> ();
+
+	  /* Namespace-scope functions can be found by ADL by template
+	     instantiations in this module.  We need to create bindings
+	     for them so that name lookup recognises they exist, if they
+	     won't be discarded.  add_binding_entity is too early to do
+	     this for GM functions, because if nobody ends up using them
+	     we'll have leftover bindings laying around, and it's tricky
+	     to delete them and any namespaces they've implicitly created
+	     deps on.  The downside is this means we don't pick up on
+	     using-decls, but by [module.global.frag] p3.6 we don't have
+	     to.  */
+	  if (ek == EK_DECL
+	      && !for_binding
+	      && !dep->is_import ()
+	      && !dep->is_tu_local ()
+	      && DECL_NAMESPACE_SCOPE_P (decl)
+	      && DECL_DECLARES_FUNCTION_P (decl)
+	      /* Compiler-generated functions won't participate in ADL.  */
+	      && !DECL_ARTIFICIAL (decl)
+	      /* A hidden friend doesn't need a binding.  */
+	      && !(DECL_LANG_SPECIFIC (not_tmpl)
+		   && DECL_UNIQUE_FRIEND_P (not_tmpl)))
+	    {
+	      /* This will only affect GM functions.  */
+	      gcc_checking_assert (!DECL_LANG_SPECIFIC (not_tmpl)
+				   || !DECL_MODULE_PURVIEW_P (not_tmpl));
+	      /* We shouldn't see any instantiations or specialisations.  */
+	      gcc_checking_assert (!DECL_LANG_SPECIFIC (decl)
+				   || !DECL_USE_TEMPLATE (decl));
+
+	      tree ns = CP_DECL_CONTEXT (decl);
+	      tree name = DECL_NAME (decl);
+	      depset *binding = find_binding (ns, name);
+	      if (!binding)
+		{
+		  binding = make_binding (ns, name);
+		  add_namespace_context (binding, ns);
+
+		  depset **slot = binding_slot (ns, name, /*insert=*/true);
+		  *slot = binding;
+		}
+
+	      binding->deps.safe_push (dep);
+	      dep->deps.safe_push (binding);
+	      dump (dumper::DEPEND)
+		&& dump ("Built ADL binding for %C:%N",
+			 TREE_CODE (decl), decl);
+	    }
 	}
 
       if (!dep->is_import ())
@@ -14488,7 +14536,10 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
 
       if ((!DECL_LANG_SPECIFIC (inner) || !DECL_MODULE_PURVIEW_P (inner))
 	  && !((flags & WMB_Using) && (flags & WMB_Purview)))
-	/* Ignore entities not within the module purview.  */
+	/* Ignore entities not within the module purview.  We'll need to
+	   create bindings for any non-discarded function calls for ADL,
+	   but it's simpler to handle that at the point of use rather
+	   than trying to clear out bindings after the fact.  */
 	return false;
 
       if (!header_module_p () && data->hash->is_tu_local_entity (decl))
@@ -14997,6 +15048,9 @@ depset::hash::add_deduction_guides (tree decl)
 
       binding->deps.safe_push (dep);
       dep->deps.safe_push (binding);
+      dump (dumper::DEPEND)
+	&& dump ("Built binding for deduction guide %C:%N",
+		 TREE_CODE (decl), decl);
     }
 }
 
diff --git a/gcc/cp/ptree.cc b/gcc/cp/ptree.cc
index 3b68c38..d074e0e 100644
--- a/gcc/cp/ptree.cc
+++ b/gcc/cp/ptree.cc
@@ -355,6 +355,7 @@ cxx_print_xnode (FILE *file, tree node, int indent)
       print_node (file, "template", TI_TEMPLATE (node), indent+4);
       print_node (file, "args", TI_ARGS (node), indent+4);
       if (TI_TEMPLATE (node)
+	  && TREE_CODE (TI_TEMPLATE (node)) == TEMPLATE_DECL
 	  && PRIMARY_TEMPLATE_P (TI_TEMPLATE (node)))
 	print_node (file, "partial", TI_PARTIAL_INFO (node), indent+4);
       if (TI_PENDING_TEMPLATE_FLAG (node))
diff --git a/gcc/testsuite/g++.dg/modules/adl-9_a.C b/gcc/testsuite/g++.dg/modules/adl-9_a.C
new file mode 100644
index 0000000..7d39f00
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-9_a.C
@@ -0,0 +1,42 @@
+// PR c++/121705
+// { dg-additional-options "-fmodules -Wno-global-module -fdump-lang-module-graph" }
+// { dg-module-cmi M }
+
+module;
+namespace helpers {
+  template <typename T> bool operator<(T, int);
+}
+namespace ns {
+  struct E {};
+  bool operator==(E, int);
+  template <typename T> bool foo(E, T);
+
+  // won't be found
+  using helpers::operator<;  // NB [module.global.frag] p3.6
+  void unused(E);
+}
+export module M;
+
+export template <typename T> bool test_op(T t, int x) {
+  // ensure it's not discarded
+  ns::E{} == x;
+  foo(ns::E{}, x);
+  // { dg-final { scan-lang-dump {Built ADL binding for function_decl:'::ns::operator=='} module } }
+  // { dg-final { scan-lang-dump {Built ADL binding for template_decl:'::ns::template foo'} module } }
+  return t == x && foo(t, x);
+}
+
+export template <typename T> bool test_using(T t, int x) {
+  // ensure it's not discarded
+  ns::E{} < 0;
+  // { dg-final { scan-lang-dump {Built ADL binding for template_decl:'::helpers::template operator<'} module } }
+  return t < x;
+}
+
+export template <typename T> void test_unused(T t) {
+  // we never use this non-dependently, so it gets discarded
+  unused(t);
+  // { dg-final { scan-lang-dump-not {'::ns::unused'} module } }
+}
+
+export using ns::E;
diff --git a/gcc/testsuite/g++.dg/modules/adl-9_b.C b/gcc/testsuite/g++.dg/modules/adl-9_b.C
new file mode 100644
index 0000000..e7898b2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-9_b.C
@@ -0,0 +1,13 @@
+// PR c++/121705
+// { dg-additional-options "-fmodules" }
+
+import M;
+int main() {
+  test_op(E{}, 0);
+
+  test_using(E{}, 0);  // { dg-message "here" }
+  // { dg-error "no match for 'operator<'" "" { target *-*-* } 0 }
+
+  test_unused(E{});  // { dg-message "here" }
+  // { dg-error "'unused' was not declared" "" { target *-*-* } 0 }
+}
diff --git a/gcc/testsuite/g++.dg/modules/default-arg-4_a.C b/gcc/testsuite/g++.dg/modules/default-arg-4_a.C
index fea1622..38e2aee 100644
--- a/gcc/testsuite/g++.dg/modules/default-arg-4_a.C
+++ b/gcc/testsuite/g++.dg/modules/default-arg-4_a.C
@@ -17,3 +17,7 @@ qux ()
 {
   return foo () + bar <int> () + baz <int> ();
 }
+
+export using ::foo;
+export using ::bar;
+export using ::baz;
diff --git a/gcc/testsuite/g++.dg/modules/default-arg-4_b.C b/gcc/testsuite/g++.dg/modules/default-arg-4_b.C
index 98b3a5f4..7ed003f 100644
--- a/gcc/testsuite/g++.dg/modules/default-arg-4_b.C
+++ b/gcc/testsuite/g++.dg/modules/default-arg-4_b.C
@@ -1,23 +1,23 @@
 // C++20 P1766R1 - Mitigating minor modules maladies
-// { dg-do run }
+// { dg-module-do run }
 // { dg-additional-options "-fmodules-ts" }
 
 import M;
 
 int
-foo (int i = 42)
+foo (int i = 42)			// { dg-bogus "default argument given for parameter 1 of 'int foo\\\(int\\\)'" "PR99000" { xfail *-*-* } }
 {
   return i;
 }
 
-template <typename T, typename U = int>
+template <typename T, typename U = int>	// { dg-bogus "redefinition of default argument for 'class U'" "PR99000" { xfail *-*-* } }
 int
 bar ()
 {
   return sizeof (U);
 }
 
-template <typename T, int N = 42>
+template <typename T, int N = 42>	// { dg-bogus "redefinition of default argument for 'int N'" "PR99000" { xfail *-*-* } }
 int
 baz ()
 {
diff --git a/gcc/testsuite/g++.dg/modules/default-arg-5_a.C b/gcc/testsuite/g++.dg/modules/default-arg-5_a.C
deleted file mode 100644
index 38e2aee..0000000
--- a/gcc/testsuite/g++.dg/modules/default-arg-5_a.C
+++ /dev/null
@@ -1,23 +0,0 @@
-// C++20 P1766R1 - Mitigating minor modules maladies
-// { dg-additional-options "-fmodules-ts -Wno-global-module" }
-// { dg-module-cmi M }
-
-module;
-
-int foo (int i = 42);
-template <typename T, typename U = int>
-int bar ();
-template <typename T, int N = 42>
-int baz ();
-
-export module M;
-
-export inline int
-qux ()
-{
-  return foo () + bar <int> () + baz <int> ();
-}
-
-export using ::foo;
-export using ::bar;
-export using ::baz;
diff --git a/gcc/testsuite/g++.dg/modules/default-arg-5_b.C b/gcc/testsuite/g++.dg/modules/default-arg-5_b.C
deleted file mode 100644
index be2c22e..0000000
--- a/gcc/testsuite/g++.dg/modules/default-arg-5_b.C
+++ /dev/null
@@ -1,35 +0,0 @@
-// C++20 P1766R1 - Mitigating minor modules maladies
-// { dg-additional-options "-fmodules-ts" }
-
-import M;
-
-int
-foo (int i = 42)			// { dg-error "default argument given for parameter 1 of 'int foo\\\(int\\\)'" }
-{
-  return i;
-}
-
-template <typename T, typename U = int>	// { dg-error "redefinition of default argument for 'class U'" }
-int
-bar ()
-{
-  return sizeof (U);
-}
-
-template <typename T, int N = 42>	// { dg-error "redefinition of default argument for 'int N'" }
-int
-baz ()
-{
-  return N;
-}
-
-int
-main ()
-{
-  if (foo () + bar <int> () + baz <int> () != qux ())
-    __builtin_abort ();
-  if (foo () != foo (42)
-      || bar <int> () != bar <int, int> ()
-      || baz <int> () != baz <int, 42> ())
-    __builtin_abort ();
-}
diff --git a/gcc/testsuite/g++.dg/modules/gmf-5.C b/gcc/testsuite/g++.dg/modules/gmf-5.C
new file mode 100644
index 0000000..a85be59
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/gmf-5.C
@@ -0,0 +1,12 @@
+// { dg-additional-options "-fmodules -Wno-global-module -fdump-lang-module" }
+// { dg-module-cmi M }
+
+module;
+namespace test {
+  struct S {};
+  void go(S);
+}
+export module M;
+
+// Ideally we don't emit any namespaces that only have discarded entries
+// { dg-final { scan-lang-dump-not {Writing namespace:[0-9]* '::test'} module } }