Re: [PATCH, RFA, C++] ICE on ill-formed template_fun(&mem_fun) within template function

[Alexandre Oliva <aoliva at redhat dot com>]

Sorry about the bad tone of my previous message on this thread.  I
guess you just caught me at a bad time, and I'm embarrassed by my
reaction.  Even if I had reasons to be frustrated by the lack of
attention to the patch, it was not fair to vent them off on you, as if
you had any obligation whatsoever of giving the patch any of your
attention, timely or otherwise.  Please accept my apologies.

On Feb 20, 2006, Mark Mitchell <mark@codesourcery.com> wrote:

> Alexandre Oliva wrote:
>> for gcc/cp/ChangeLog
>> from  Alexandre Oliva  <aoliva@redhat.com>
>> 
>> * class.c (instantiate_type): Handle NON_DEPENDENT_EXPR.
>> * pt.c (resolve_overloaded_unification): Likewise.
>> (build_non_dependent_expr): Remove incorrect assertion.
>> * semantics.c (finish_non_static_data_member): Don't mark
>> current_class_ptr as used if it's NULL.
>> 
>> for gcc/testsuite/ChangeLog
>> from  Alexandre Oliva  <aoliva@redhat.com>
>> 
>> * g++.dg/template/dependent-expr5.C: New test.

> This needs a lot more explanation.  It looks like you may be fixing
> several things; in that case, let's explain each thing separately.

Without the changes in class.c, taking the address of a static member
function passed as an argument to a template function call within a
template using an unqualified name doesn't work: overload resolution
fails for the call in line 92.

Without the changes in pt.c, taking the address of a static member
function passed as an argument to a template function call within a
template using an unqualified name doesn't work: we crash in overload
resolution compiling line 86 of the testcase.  This is only slightly
different from the above, in that the function called in this line
fails overload resolution, whereas in the above it should work.

>> The semantics.c change looks like it might be
>> independent of the other bits?

It was necessary to get the testcase to compile with an earlier
version of GCC, but it appears to no longer be necessary in the trunk,
at least for the testcase I have posted.  I can take it out if you
feel it is not correct.  It looked reasonable at the time I added it.

> Do we really need to pass
> NON_DEPENDENT_EXPRs down into instantiate_type?

We need to do this in order to resolve the unknown type resulting from
taking the address of barf, passed as argument to bindn() in line 92
of the testcase.

The name is clearly non-dependent.

When the set of overloads involves only non-member functions,
build_non_dependent_expr for the addr_expr manages to avoid the
creation of the non_dependent_expr because the operand of ADDR_EXPR is
an overloaded function.

However, when there's any non-member function found by looking up the
name, finish_id_expression () creates a COMPONENT_REF expression, and
then when we build_non_dependent_expr of an ADDR_EXPR of a
COMPONENT_REF, the code that avoids creating the non_dependent_expr
for ADDR_EXPRs and COMPONENT_REFs of overloads doesn't match, so we
end up with the (undesired?) NON_DEPENDENT_EXPR.

> That's very scary, and if the only reason is to get an error
> message, I'd hope we could do it sooner.

It's not.  This is necessary in order to compile line 92 correctly.

This alternate, much simpler patch, appears to fix the problem in
mainline, by extending the code that matches ADDR_EXPR and
COMPONENT_REF to avoid wrapping them in a NON_DEPENDENT_EXPR if their
inner_expr is an overloaded function, to match ADDR_EXPR of
COMPONENT_REF as well.  I'm still testing it, but does it look
like it might be a better fix for the problem?

Red Hat bugzilla #168260 triggered an ICE on an ill-formed construct,
taking the address of a non-static member function using an
unqualified id, within a template function, and passing that as an
argument in a function call requiring overload resolution.  Fixing
that uncovered a number of other situations in which we used to fail
or to handle things differently within templates.

for gcc/cp/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* pt.c (build_non_dependent_expr): Leave ADDR_EXPR of
	COMPONENT_REF alone.

for gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* g++.dg/template/dependent-expr5.C: New test.

Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c.orig	2006-03-28 03:26:53.000000000 -0300
+++ gcc/cp/pt.c	2006-03-29 04:54:25.000000000 -0300
@@ -12894,10 +12894,11 @@ build_non_dependent_expr (tree expr)
     return expr;
   /* Preserve OVERLOADs; the functions must be available to resolve
      types.  */
-  inner_expr = (TREE_CODE (expr) == ADDR_EXPR ?
-		TREE_OPERAND (expr, 0) :
-		TREE_CODE (expr) == COMPONENT_REF ?
-		TREE_OPERAND (expr, 1) : expr);
+  inner_expr = expr;
+  if (TREE_CODE (inner_expr) == ADDR_EXPR)
+    inner_expr = TREE_OPERAND (inner_expr, 0);
+  if (TREE_CODE (inner_expr) == COMPONENT_REF)
+    inner_expr = TREE_OPERAND (inner_expr, 1);
   if (is_overloaded_fn (inner_expr)
       || TREE_CODE (inner_expr) == OFFSET_REF)
     return expr;
Index: gcc/testsuite/g++.dg/template/dependent-expr5.C
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/g++.dg/template/dependent-expr5.C	2006-03-29 04:29:34.000000000 -0300
@@ -0,0 +1,114 @@
+// { dg-do compile }
+
+// Copyright 2005 Free Software Foundation
+// contributed by Alexandre Oliva <aoliva@redhat.com>
+// inspired in the failure reported in Red Hat bugzilla #168260.
+
+template<class F> void bind(F f) {}
+
+template<class F> void bindm(F f) {}
+template<class F, class T> void bindm(F (T::*f)(void)) {} // { dg-error "note" }
+
+template<class F> void bindn(F f) {}
+template<class F, class T> void bindn(F (*f)(T)) {}
+
+template<class F> void bindb(F f) {}
+template<class F, class T> void bindb(F (*f)(T)) {} // { dg-error "note" }
+template<class F, class T> void bindb(F (T::*f)(void)) {} // { dg-error "note" }
+
+struct foo {
+  static int baist;
+  int bait;
+  void barf ();
+  static void barf (int);
+
+  struct bar {
+    static int baikst;
+    int baikt;
+    void bark ();
+    static void bark (int);
+
+    bar() {
+      bind (&baist);
+      bind (&foo::baist);
+      bind (&bait); // { dg-error "nonstatic data member" }
+      bind (&foo::bait);
+
+      bind (&baikst);
+      bind (&bar::baikst);
+      bind (&baikt); // ok, this->baikt
+      bind (&bar::baikt);
+
+      bind (&barf); // { dg-error "no matching function" }
+      bind (&foo::barf); // { dg-error "no matching function" }
+
+      bindm (&barf); // { dg-error "no matching function" }
+      bindm (&foo::barf);
+
+      bindn (&barf);
+      bindn (&foo::barf);
+
+      bindb (&barf);
+      bindb (&foo::barf); // { dg-error "ambiguous" }
+
+      bind (&bark); // { dg-error "no matching function" }
+      bind (&bar::bark); // { dg-error "no matching function" }
+
+      bindm (&bark); // { dg-error "no matching function" }
+      bindm (&bar::bark);
+
+      bindn (&bark);
+      bindn (&bar::bark);
+
+      bindb (&bark);
+      bindb (&bar::bark); // { dg-error "ambiguous" }
+    }
+  };
+
+  template <typename T>
+  struct barT {
+    static int baikst;
+    int baikt;
+    void bark ();
+    static void bark (int);
+
+    barT() {
+      bind (&baist);
+      bind (&foo::baist);
+      bind (&bait); // { dg-error "nonstatic data member" }
+      bind (&foo::bait);
+
+      bind (&baikst);
+      bind (&barT::baikst);
+      bind (&baikt); // ok, this->baikt
+      bind (&barT::baikt);
+
+      bind (&barf); // { dg-error "no matching function" }
+      bind (&foo::barf); // { dg-error "no matching function" }
+
+      bindm (&barf); // { dg-error "no matching function" }
+      bindm (&foo::barf);
+
+      bindn (&barf);
+      bindn (&foo::barf);
+
+      bindb (&barf);
+      bindb (&foo::barf); // { dg-error "ambiguous" }
+
+      bind (&bark); // { dg-error "no matching function" }
+      bind (&barT::bark); // { dg-error "no matching function" }
+
+      bindm (&bark); // { dg-error "no matching function" }
+      bindm (&barT::bark);
+
+      bindn (&bark);
+      bindn (&barT::bark);
+
+      bindb (&bark);
+      bindb (&barT::bark); // { dg-error "ambiguous" }
+    }
+  };
+
+  bar bard;
+  barT<void> bart;
+} bad;

-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Secretary for FSF Latin America        http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}