diff mbox

C++ PATCH for c++/69753 (DR141 broke member template lookup)

Message ID 56C23F88.5050109@redhat.com
State New
Headers show

Commit Message

Jason Merrill Feb. 15, 2016, 9:13 p.m. UTC 
When we stopped finding function templates with unqualified lookup due 
to the DR141 fix, that exposed bugs in our lookup within the object 
expression scope; an object-expression of the current instantiation does 
not make the expression dependent.  This patch fixes this issue 
specifically for implicit "this->", which is the case in question in 
this PR; addressing this issue more generally will take more work.

Tested x86_64-pc-linux-gnu, applying to trunk.

Comments

Markus Trippelsdorf Feb. 16, 2016, 10:38 a.m. UTC | #1 
On 2016.02.15 at 16:13 -0500, Jason Merrill wrote:
> When we stopped finding function templates with unqualified lookup due to
> the DR141 fix, that exposed bugs in our lookup within the object expression
> scope; an object-expression of the current instantiation does not make the
> expression dependent.  This patch fixes this issue specifically for implicit
> "this->", which is the case in question in this PR; addressing this issue
> more generally will take more work.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk.

> diff --git a/gcc/testsuite/g++.dg/lookup/member3.C b/gcc/testsuite/g++.dg/lookup/member3.C
> new file mode 100644
> index 0000000..f4e097e4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/member3.C
> @@ -0,0 +1,17 @@
> +// PR c++/69753
> +// { dg-do compile { target c++11 } }
> +
> +class A {
> +public:
> +  template <typename> void As();
> +  static A *FromWebContents();
> +  A *FromWebContents2();
> +};
> +template <typename T> class B : A {
> +  void FromWebContents() {
> +    auto guest = A::FromWebContents();
> +    guest ? guest->As<T>() : nullptr;
> +    auto guest2 = A::FromWebContents2();
> +    guest2 ? guest2->As<T>() : nullptr;
> +  }
> +};

Please note that clang rejects the testcase in the non static case:

gcc/testsuite/g++.dg/lookup/member3.C:15:22: error: use 'template' keyword to treat 'As' as a dependent template name
    guest2 ? guest2->As<T>() : nullptr;
                     ^
                     template 
1 error generated.

Here is another testcase that every compiler I've tested (clang, icc,
microsoft) accepts, but is rejected by gcc-6:

class A {
public:
  template <class> void m_fn1();
};
A *fn1(int *);
template <typename> class B : A {
  static int *m_fn2() { fn1(m_fn2())->m_fn1<A>(); }
};
Patrick Palka Feb. 16, 2016, 2:32 p.m. UTC | #2 
On Tue, Feb 16, 2016 at 5:38 AM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2016.02.15 at 16:13 -0500, Jason Merrill wrote:
>> When we stopped finding function templates with unqualified lookup due to
>> the DR141 fix, that exposed bugs in our lookup within the object expression
>> scope; an object-expression of the current instantiation does not make the
>> expression dependent.  This patch fixes this issue specifically for implicit
>> "this->", which is the case in question in this PR; addressing this issue
>> more generally will take more work.
>>
>> Tested x86_64-pc-linux-gnu, applying to trunk.
>
>> diff --git a/gcc/testsuite/g++.dg/lookup/member3.C b/gcc/testsuite/g++.dg/lookup/member3.C
>> new file mode 100644
>> index 0000000..f4e097e4
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lookup/member3.C
>> @@ -0,0 +1,17 @@
>> +// PR c++/69753
>> +// { dg-do compile { target c++11 } }
>> +
>> +class A {
>> +public:
>> +  template <typename> void As();
>> +  static A *FromWebContents();
>> +  A *FromWebContents2();
>> +};
>> +template <typename T> class B : A {
>> +  void FromWebContents() {
>> +    auto guest = A::FromWebContents();
>> +    guest ? guest->As<T>() : nullptr;
>> +    auto guest2 = A::FromWebContents2();
>> +    guest2 ? guest2->As<T>() : nullptr;
>> +  }
>> +};
>
> Please note that clang rejects the testcase in the non static case:
>
> gcc/testsuite/g++.dg/lookup/member3.C:15:22: error: use 'template' keyword to treat 'As' as a dependent template name
>     guest2 ? guest2->As<T>() : nullptr;
>                      ^
>                      template
> 1 error generated.
>
> Here is another testcase that every compiler I've tested (clang, icc,
> microsoft) accepts, but is rejected by gcc-6:
>
> class A {
> public:
>   template <class> void m_fn1();
> };
> A *fn1(int *);
> template <typename> class B : A {
>   static int *m_fn2() { fn1(m_fn2())->m_fn1<A>(); }
> };
>
> --
> Markus

clang also rejects the case where A::FromWebContents is overloaded
with a static member function and non-static one, and gcc now accepts
this case.

class A {
public:
  template <typename> void As();
  static A *FromWebContents();
  template <typename... Ts>
  A *FromWebContents(Ts...);
};
template <typename T> class B : A {
  void FromWebContents() {
    auto guest = A::FromWebContents();
    guest->As<T>();
  }
};
Jason Merrill Feb. 16, 2016, 6:26 p.m. UTC | #3 
On 02/16/2016 09:32 AM, Patrick Palka wrote:
> clang also rejects the case where A::FromWebContents is overloaded
> with a static member function and non-static one, and gcc now accepts
> this case.

This is a clang bug.

Jason
Jason Merrill Feb. 17, 2016, 4:22 a.m. UTC | #4 
On 02/16/2016 05:38 AM, Markus Trippelsdorf wrote:
> Here is another testcase that every compiler I've tested (clang, icc,
> microsoft) accepts, but is rejected by gcc-6:
>
> class A {
> public:
>    template <class> void m_fn1();
> };
> A *fn1(int *);
> template <typename> class B : A {
>    static int *m_fn2() { fn1(m_fn2())->m_fn1<A>(); }
> };

Fixed.

Jason
diff mbox

Patch

commit 9a196a7306eb2c0b6eed5e81dd2d5a65331347bb
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Feb 11 11:51:03 2016 -0500

    	PR c++/69753
    
    	* search.c (any_dependent_bases_p): Split out...
    	* name-lookup.c (do_class_using_decl): ...from here.
    	* call.c (build_new_method_call_1): Don't complain about missing object
    	if there are dependent bases.  Tweak error.
    	* tree.c (non_static_member_function_p): Remove.
    	* pt.c (type_dependent_expression_p): A member template of a
    	dependent type is dependent.
    	* cp-tree.h: Adjust.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index cb71176..db40654 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8160,7 +8160,7 @@  build_new_method_call_1 (tree instance, tree fns, vec<tree, va_gc> **args,
 
       if (permerror (input_location,
 		     "cannot call constructor %<%T::%D%> directly",
-		     basetype, name))
+		     BINFO_TYPE (access_binfo), name))
 	inform (input_location, "for a function-style cast, remove the "
 		"redundant %<::%D%>", name);
       call = build_functional_cast (basetype, build_tree_list_vec (user_args),
@@ -8377,6 +8377,9 @@  build_new_method_call_1 (tree instance, tree fns, vec<tree, va_gc> **args,
 		     we know we really need it.  */
 		  cand->first_arg = instance;
 		}
+	      else if (any_dependent_bases_p ())
+		/* We can't tell until instantiation time whether we can use
+		   *this as the implicit object argument.  */;
 	      else
 		{
 		  if (complain & tf_error)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3b91089..b7d7bc6 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6252,6 +6252,7 @@  extern tree adjust_result_of_qualified_name_lookup
 extern tree copied_binfo			(tree, tree);
 extern tree original_binfo			(tree, tree);
 extern int shared_member_p			(tree);
+extern bool any_dependent_bases_p (tree = current_nonlambda_class_type ());
 
 /* The representation of a deferred access check.  */
 
@@ -6542,7 +6543,6 @@  extern tree get_first_fn			(tree);
 extern tree ovl_cons				(tree, tree);
 extern tree build_overload			(tree, tree);
 extern tree ovl_scope				(tree);
-extern bool non_static_member_function_p        (tree);
 extern const char *cxx_printable_name		(tree, int);
 extern const char *cxx_printable_name_translate	(tree, int);
 extern tree build_exception_variant		(tree, tree);
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 8d6e75a..b5961e5 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -3333,8 +3333,6 @@  do_class_using_decl (tree scope, tree name)
   /* True if any of the bases of CURRENT_CLASS_TYPE are dependent.  */
   bool bases_dependent_p;
   tree binfo;
-  tree base_binfo;
-  int i;
 
   if (name == error_mark_node)
     return NULL_TREE;
@@ -3371,16 +3369,7 @@  do_class_using_decl (tree scope, tree name)
 		      || (IDENTIFIER_TYPENAME_P (name)
 			  && dependent_type_p (TREE_TYPE (name))));
 
-  bases_dependent_p = false;
-  if (processing_template_decl)
-    for (binfo = TYPE_BINFO (current_class_type), i = 0;
-	 BINFO_BASE_ITERATE (binfo, i, base_binfo);
-	 i++)
-      if (dependent_type_p (TREE_TYPE (base_binfo)))
-	{
-	  bases_dependent_p = true;
-	  break;
-	}
+  bases_dependent_p = any_dependent_bases_p (current_class_type);
 
   decl = NULL_TREE;
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a55dc10..52e60b9 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -22904,9 +22904,16 @@  type_dependent_expression_p (tree expression)
       && DECL_TEMPLATE_INFO (expression))
     return any_dependent_template_arguments_p (DECL_TI_ARGS (expression));
 
-  if (TREE_CODE (expression) == TEMPLATE_DECL
-      && !DECL_TEMPLATE_TEMPLATE_PARM_P (expression))
-    return false;
+  if (TREE_CODE (expression) == TEMPLATE_DECL)
+    {
+      if (DECL_CLASS_SCOPE_P (expression)
+	  && dependent_type_p (DECL_CONTEXT (expression)))
+	/* A template's own parameters don't make it dependent, since those can
+	   be deduced, but the enclosing class does.  */
+	return true;
+      if (!DECL_TEMPLATE_TEMPLATE_PARM_P (expression))
+	return false;
+    }
 
   if (TREE_CODE (expression) == STMT_EXPR)
     expression = stmt_expr_value_expr (expression);
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index 7924611..49f3bc5 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -2842,3 +2842,21 @@  original_binfo (tree binfo, tree here)
   return result;
 }
 
+/* True iff TYPE has any dependent bases (and therefore we can't say
+   definitively that another class is not a base of an instantiation of
+   TYPE).  */
+
+bool
+any_dependent_bases_p (tree type)
+{
+  if (!type || !CLASS_TYPE_P (type) || !processing_template_decl)
+    return false;
+
+  unsigned i;
+  tree base_binfo;
+  FOR_EACH_VEC_SAFE_ELT (BINFO_BASE_BINFOS (TYPE_BINFO (type)), i, base_binfo)
+    if (BINFO_DEPENDENT_BASE_P (base_binfo))
+      return true;
+
+  return false;
+}
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 0cf5f93..0f6a6b5 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2264,17 +2264,7 @@  finish_call_expr (tree fn, vec<tree, va_gc> **args, bool disallow_virtual,
 	 with no type; type_dependent_expression_p recognizes
 	 expressions with no type as being dependent.  */
       if (type_dependent_expression_p (fn)
-	  || any_type_dependent_arguments_p (*args)
-	  /* For a non-static member function that doesn't have an
-	     explicit object argument, we need to specifically
-	     test the type dependency of the "this" pointer because it
-	     is not included in *ARGS even though it is considered to
-	     be part of the list of arguments.  Note that this is
-	     related to CWG issues 515 and 1005.  */
-	  || (TREE_CODE (fn) != COMPONENT_REF
-	      && non_static_member_function_p (fn)
-	      && current_class_ref
-	      && type_dependent_expression_p (current_class_ref)))
+	  || any_type_dependent_arguments_p (*args))
 	{
 	  result = build_nt_call_vec (fn, *args);
 	  SET_EXPR_LOCATION (result, EXPR_LOC_OR_LOC (fn, input_location));
@@ -2354,17 +2344,6 @@  finish_call_expr (tree fn, vec<tree, va_gc> **args, bool disallow_virtual,
       object = maybe_dummy_object (BINFO_TYPE (BASELINK_ACCESS_BINFO (fn)),
 				   NULL);
 
-      if (processing_template_decl)
-	{
-	  if (type_dependent_expression_p (object))
-	    {
-	      tree ret = build_nt_call_vec (orig_fn, orig_args);
-	      release_tree_vector (orig_args);
-	      return ret;
-	    }
-	  object = build_non_dependent_expr (object);
-	}
-
       result = build_new_method_call (object, fn, args, NULL_TREE,
 				      (disallow_virtual
 				       ? LOOKUP_NORMAL|LOOKUP_NONVIRTUAL
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 3203aca..041facd 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -2071,23 +2071,6 @@  ovl_scope (tree ovl)
     ovl = OVL_CHAIN (ovl);
   return CP_DECL_CONTEXT (OVL_CURRENT (ovl));
 }
-
-/* Return TRUE if FN is a non-static member function, FALSE otherwise.
-   This function looks into BASELINK and OVERLOAD nodes.  */
-
-bool
-non_static_member_function_p (tree fn)
-{
-  if (fn == NULL_TREE)
-    return false;
-
-  if (is_overloaded_fn (fn))
-    fn = get_first_fn (fn);
-
-  return (DECL_P (fn)
-	  && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn));
-}
-
 
 #define PRINT_RING_SIZE 4
 
diff --git a/gcc/testsuite/g++.dg/lookup/member3.C b/gcc/testsuite/g++.dg/lookup/member3.C
new file mode 100644
index 0000000..f4e097e4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/member3.C
@@ -0,0 +1,17 @@ 
+// PR c++/69753
+// { dg-do compile { target c++11 } }
+
+class A {
+public:
+  template <typename> void As();
+  static A *FromWebContents();
+  A *FromWebContents2();
+};
+template <typename T> class B : A {
+  void FromWebContents() {
+    auto guest = A::FromWebContents();
+    guest ? guest->As<T>() : nullptr;
+    auto guest2 = A::FromWebContents2();
+    guest2 ? guest2->As<T>() : nullptr;
+  }
+};