Patchwork Fix PR c++/47172

login
register
mail settings
Submitter Dodji Seketeli
Date Feb. 10, 2011, 12:49 p.m.
Message ID <m3ipwsdph1.fsf@redhat.com>
Download mbox | patch
Permalink /patch/82603/
State New
Headers show

Comments

Dodji Seketeli - Feb. 10, 2011, 12:49 p.m.
After we discussed this on IRC I updated the patch to make
finish_call_expr recognize non-static member call expression [which
id-expression is non dependent but] which implicit "this" pointer is
dependent as being dependent; as a result the call expression is built
with no type. type_dependent_expression_p then later recognizes that
expression with no type as being dependent.

I have added comments to other parts that didn't appear self evident to
me while looking at this.

Tested on x86-64-unknown-linux-gnu against trunk.
Gabriel Dos Reis - Feb. 10, 2011, 2:59 p.m.
On Thu, Feb 10, 2011 at 6:49 AM, Dodji Seketeli <dodji@redhat.com> wrote:

> +      /* If the call expression is dependent, build a CALL_EXPR node
> +        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))
> +         || any_type_dependent_arguments_p (*args)
> +         /* In a template scope, a call expression which
> +            id-expression is non-dependent is considered dependent if
> +            the implicit "this" is dependent. Note that the
> +            id-expression is already bound (at template definition
> +            time) so this respects the essence of the two-phases name
> +            lookup.  */

Note that there is a strong disagreement/debate in the CWG about this.
 So, putting it in
the C++ front-end source code does not necssarily help :-)
Dodji Seketeli - Feb. 10, 2011, 3:26 p.m.
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> Note that there is a strong disagreement/debate in the CWG about this.
> So, putting it in the C++ front-end source code does not necssarily
> help :-)

OK. I just added the comment b/c it didn't seem obvious to me at first
sight. I didn't mean to add to the controversy. I can remove it.
Jason Merrill - Feb. 10, 2011, 3:44 p.m.
On 02/10/2011 10:26 AM, Dodji Seketeli wrote:
> Gabriel Dos Reis<gdr@integrable-solutions.net>  writes:
>
>> Note that there is a strong disagreement/debate in the CWG about this.
>> So, putting it in the C++ front-end source code does not necssarily
>> help :-)
>
> OK. I just added the comment b/c it didn't seem obvious to me at first
> sight. I didn't mean to add to the controversy. I can remove it.

Just add a reference to issues 515 and 1005.  For 4.6 we just want to 
restore the previous behavior.

> +	  || (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
> +	      && current_class_ref
> +	      && type_dependent_expression_p (current_class_ref)))

I'm uncomfortable with checking DECL_NONSTATIC_MEMBER_FUNCTION_P before 
we've established that fn is a FUNCTION_DECL.  Let's check its TREE_CODE 
first.

>  /* Returns TRUE if the EXPRESSION is type-dependent, in the sense of
> -   [temp.dep.expr].  */
> +   [temp.dep.expr]. Note that an expression with no type is
> +   considered dependent.  */

Let's also note that other parts of the compiler arrange for an 
expression with type-dependent subexpressions to have no type, so we 
don't need to recurse in type_dependent_expression_p.

Jason
Gabriel Dos Reis - Feb. 10, 2011, 6:31 p.m.
On Thu, Feb 10, 2011 at 9:26 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>
>> Note that there is a strong disagreement/debate in the CWG about this.
>> So, putting it in the C++ front-end source code does not necssarily
>> help :-)
>
> OK. I just added the comment b/c it didn't seem obvious to me at first
> sight. I didn't mean to add to the controversy. I can remove it.

the point is that there is a disagreemnt that a source expression that
is not dependent should suddenly become dependent -- in fact, it is not.
So, while I agree that a documentation of logic is welcome, this particular
adds confusion.
I understand that GCC is doing something (that you would like to document
clearly so that future developers don't waste precious time trying to
"reverse engineer" the ideas), however, there is a debate whether GCC
should be doing what it is doing.

Patch

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index d59f32a..9811f4a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17912,7 +17912,7 @@  dependent_type_p_r (tree type)
 }
 
 /* Returns TRUE if TYPE is dependent, in the sense of
-   [temp.dep.type].  */
+   [temp.dep.type]. Note that a NULL type is considered dependent.  */
 
 bool
 dependent_type_p (tree type)
@@ -18184,7 +18184,8 @@  value_dependent_expression_p (tree expression)
 }
 
 /* Returns TRUE if the EXPRESSION is type-dependent, in the sense of
-   [temp.dep.expr].  */
+   [temp.dep.expr]. Note that an expression with no type is
+   considered dependent.  */
 
 bool
 type_dependent_expression_p (tree expression)
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 6d45fb9..2d71e95 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2028,8 +2028,20 @@  finish_call_expr (tree fn, VEC(tree,gc) **args, bool disallow_virtual,
 
   if (processing_template_decl)
     {
+      /* If the call expression is dependent, build a CALL_EXPR node
+	 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))
+	  || any_type_dependent_arguments_p (*args)
+	  /* In a template scope, a call expression which
+	     id-expression is non-dependent is considered dependent if
+	     the implicit "this" is dependent. Note that the
+	     id-expression is already bound (at template definition
+	     time) so this respects the essence of the two-phases name
+	     lookup.  */
+	  || (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
+	      && current_class_ref
+	      && type_dependent_expression_p (current_class_ref)))
 	{
 	  result = build_nt_call_vec (fn, *args);
 	  KOENIG_LOOKUP_P (result) = koenig_p;
diff --git a/gcc/testsuite/g++.dg/template/inherit6.C b/gcc/testsuite/g++.dg/template/inherit6.C
new file mode 100644
index 0000000..241a68e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/inherit6.C
@@ -0,0 +1,23 @@ 
+// Origin PR c++/47172
+// { dg-options "-std=c++0x" }
+// { dg-do compile }
+
+struct A
+{
+    int f() const;
+};
+
+template <class T>
+struct B : A { };
+
+template <class T>
+struct C : B<T>
+{
+    void g();
+};
+
+template <class T>
+void C<T>::g()
+{
+    A::f();
+}