diff mbox

Fix PR c++/45383

Message ID m3mxpggtk3.fsf@redhat.com
State New
Headers show

Commit Message

Dodji Seketeli Nov. 11, 2010, 8:20 a.m. UTC 
Hello,

In the exemple gcc/testsuite/g++.dg/conversion/cond4.C of the second
patch below, G++ fails to consider the conversion function template
template<class T> operator T*() of struct null so it fails to compile
the line if (ptr == null).

This is due to my fix for PR c++/42260. In that bug the following code
was producing and ICE:

struct A
{
      template<typename T> operator T*(); //#1
};

int i = *A();// <- ICE here.

The ICE was because in build_new_op [while looking at the expression
*A()] add_builtin_candidates was considering the conversion function
template #1 as a candidate for user conversion and was eventually
wrongly choosing it. I thought I could fix the problem by avoiding
considering function templates when looking for candidate conversion
functions.

This current bug seems to argue that systematically ruling out
function templates from the list of candidates in
add_builtin_candidate was probably too strong of a measure.

Furthermore, a comment of add_builtin_candidates reads:

   Here we generate a superset of the possible candidates for this
   particular case.  That is a subset of the full set the standard
   defines, plus some other cases which the standard
   disallows. add_builtin_candidate will filter out the invalid set.

add_builtin_candidates seems to be designed to first generate a
superset of the possible candidates and then let
add_building_candidate (without 's' this time) rule out some invalid
candidates.

So I went to look at why add_building_candidate wasn't filtering out the
conversion function template.

I suppose that for a conversion function 'T& operator*(T*)', T should
be a complete type [the comment of that case in the
add_building_candidate even says that too]. But the code doesn't
enforce that. If we were looking at a type dependent expression, I
guess this whole overloading business would be deferred to after the
dependent expression is tsubsted.

Incidentally, requiring that T be complete in that context is also similar to what
add_building_candidate does for the operator
CV12 T& operator->*(CV1 C1*, CV2 T C2::*).

So the first patch below is a revert of the initial patch for PR
c++/42260. The revert fixes this PR c++/45383.

The second patch makes add_building_candidate require that the result
of applying the operator T& operator*(T*) be a complete type, and that
fixes PR c++/42260.

I have bootstrapped and regtested both patches together on
x86-unknown-linux gnu against trunk and I am about to do it for 4.5
too if the approach is OK.

What do you think?

Comments

Jason Merrill Nov. 24, 2010, 11:05 p.m. UTC | #1 
On 11/11/2010 03:20 AM, Dodji Seketeli wrote:
> I suppose that for a conversion function 'T&  operator*(T*)', T should
> be a complete type [the comment of that case in the
> add_building_candidate even says that too].

Hmm, I guess you're right.  OK.

Jason
diff mbox

Patch

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index eb7247d..bf9439f 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -1093,7 +1093,7 @@  convert_class_to_reference (tree reference_type, tree s, tree expr, int flags)
   if (!expr)
     return NULL;
 
-  conversions = lookup_conversions (s, /*lookup_template_convs_p=*/true);
+  conversions = lookup_conversions (s);
   if (!conversions)
     return NULL;
 
@@ -2464,8 +2464,7 @@  add_builtin_candidates (struct z_candidate **candidates, enum tree_code code,
 	  if (i == 0 && code == MODIFY_EXPR && code2 == NOP_EXPR)
 	    return;
 
-	  convs = lookup_conversions (argtypes[i],
-				      /*lookup_template_convs_p=*/false);
+	  convs = lookup_conversions (argtypes[i]);
 
 	  if (code == COND_EXPR)
 	    {
@@ -3028,8 +3027,7 @@  build_user_type_conversion_1 (tree totype, tree expr, int flags)
 	     reference to it)...  */
 	}
       else
-	conv_fns = lookup_conversions (fromtype,
-				       /*lookup_template_convs_p=*/true);
+	conv_fns = lookup_conversions (fromtype);
     }
 
   candidates = 0;
@@ -3585,7 +3583,7 @@  build_op_call (tree obj, VEC(tree,gc) **args, tsubst_flags_t complain)
 		      LOOKUP_NORMAL, &candidates);
     }
 
-  convs = lookup_conversions (type, /*lookup_template_convs_p=*/true);
+  convs = lookup_conversions (type);
 
   for (; convs; convs = TREE_CHAIN (convs))
     {
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 241805c..2a52742 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5161,7 +5161,7 @@  extern int at_function_scope_p			(void);
 extern bool at_class_scope_p			(void);
 extern bool at_namespace_scope_p		(void);
 extern tree context_for_name_lookup		(tree);
-extern tree lookup_conversions			(tree, bool);
+extern tree lookup_conversions			(tree);
 extern tree binfo_from_vbase			(tree);
 extern tree binfo_for_vbase			(tree, tree);
 extern tree look_for_overrides_here		(tree, tree);
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index d2d6f4a..17ba540 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -1476,9 +1476,7 @@  build_expr_type_conversion (int desires, tree expr, bool complain)
   if (!TYPE_HAS_CONVERSION (basetype))
     return NULL_TREE;
 
-  for (conv = lookup_conversions (basetype, /*lookup_template_convs_p=*/true);
-       conv;
-       conv = TREE_CHAIN (conv))
+  for (conv = lookup_conversions (basetype); conv; conv = TREE_CHAIN (conv))
     {
       int win = 0;
       tree candidate;
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index 370ddf6..c02800c 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -2440,13 +2440,10 @@  lookup_conversions_r (tree binfo,
    functions in this node were selected.  This function is effectively
    performing a set of member lookups as lookup_fnfield does, but
    using the type being converted to as the unique key, rather than the
-   field name.
-   If LOOKUP_TEMPLATE_CONVS_P is TRUE, the returned TREE_LIST contains
-   the non-hidden user-defined template conversion functions too.  */
+   field name.  */
 
 tree
-lookup_conversions (tree type,
-		    bool lookup_template_convs_p)
+lookup_conversions (tree type)
 {
   tree convs, tpl_convs;
   tree list = NULL_TREE;
@@ -2473,9 +2470,6 @@  lookup_conversions (tree type,
 	}
     }
 
-  if (lookup_template_convs_p == false)
-    tpl_convs = NULL_TREE;
-
   for (; tpl_convs; tpl_convs = TREE_CHAIN (tpl_convs))
     {
       tree probe, next;
diff --git a/gcc/testsuite/g++.dg/conversion/cast2.C b/gcc/testsuite/g++.dg/conversion/cast2.C
deleted file mode 100644
index 3868d74..0000000
--- a/gcc/testsuite/g++.dg/conversion/cast2.C
+++ /dev/null
@@ -1,11 +0,0 @@ 
-// Contributed by Dodji Seketeli <dodji@redhat.com>
-// Origin: PR c++/42260
-// { dg-do compile }
-
-struct A
-{
-      template<typename T> operator T*();
-};
-
-int i = *A();// { dg-error "no match" }
-
-- 
1.7.2.3


From 6833062fd785fdaf8147b19f29a131ec8705d421 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <dodji@seketeli.org>
Date: Wed, 10 Nov 2010 16:02:12 +0100
Subject: [PATCH 2/2] Fix PR c++/42260 and ensure PR c++/45383 is fixed

gcc/cp/
	c++/42260
	* call.c (add_builtin_candidate): At this point the resulting type
	of an indirection operator should be complete.

gcc/testsuite/
	c++/42260
	c++/45383
	* g++.dg/conversion/cast2.C: New test.
	* g++.dg/conversion/cond4/C: Likewise. Ensures we don't regress on
	PR c++/45383
---
 gcc/cp/call.c                           |    1 +
 gcc/testsuite/g++.dg/conversion/cast2.C |    9 +++++++++
 gcc/testsuite/g++.dg/conversion/cond4.C |   31 +++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 0 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/conversion/cast2.C
 create mode 100644 gcc/testsuite/g++.dg/conversion/cond4.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index bf9439f..12d55b5 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -2022,6 +2022,7 @@  add_builtin_candidate (struct z_candidate **candidates, enum tree_code code,
 
     case INDIRECT_REF:
       if (TREE_CODE (type1) == POINTER_TYPE
+	  && is_complete (TREE_TYPE (type1))
 	  && (TYPE_PTROB_P (type1)
 	      || TREE_CODE (TREE_TYPE (type1)) == FUNCTION_TYPE))
 	break;
diff --git a/gcc/testsuite/g++.dg/conversion/cast2.C b/gcc/testsuite/g++.dg/conversion/cast2.C
new file mode 100644
index 0000000..ac83297
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/cast2.C
@@ -0,0 +1,9 @@ 
+// Origin: PR c++/42260
+// { dg-do compile }
+
+struct A
+{
+      template<typename T> operator T*();
+};
+
+int i = *A();// { dg-error "no match" }
diff --git a/gcc/testsuite/g++.dg/conversion/cond4.C b/gcc/testsuite/g++.dg/conversion/cond4.C
new file mode 100644
index 0000000..3bd6476
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/cond4.C
@@ -0,0 +1,31 @@ 
+// Origin: PR c++/45383
+// { dg-do run }
+
+struct null {
+    null() {}
+    template<class T>
+    operator T*() const {
+    return 0;
+    }
+
+    template<class C, class T>
+    operator T C::*() const {
+    return 0;
+    }
+private:
+    null(const null&);
+    null& operator=(const null&);
+    void operator&() const;
+};
+
+static struct null null;
+
+int
+main()
+{
+    int* ptr = null;
+    if (ptr == null)
+        return 0;
+    if (ptr != null)
+        return 1;
+}