[PATCH] Fix PR c++/26698: g++ accepts const-incorrect code due to conversion function

[PATCH] Fix PR c++/26698: g++ accepts const-incorrect code due to conversion function

[Simon Martin]

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

Hi all.

This PR was opened because we would not handle the following snippet
properly:

=== cut here ===
struct X {
        int x;
        X ( int i = 0 )
          : x ( i )
        {}
        operator X & ( void ) const {
          return ( *this );
        }
};
void add_one ( X & ref ) {
        ++ ref.x;
}
int main ( void ) {
        X const a ( 2 );
        add_one( a );
}
=== cut here ===

At the time this PR was opened, we would accept it without complaining,
while there are two problems:
       1. The body of "operator X&" is invalid because it returns a
non-const reference to the const object it's called on.
       2. This operator should not be considered when calling add_one,
because the standard states that "A conversion function is never used to
convert a (possibly cv-qualified) object to the (possibly cv-qualified)
same object type (or a reference to it), to a (possibly cv-qualified)
base class of that type (or a reference to it)".

The first point is now properly rejected, but the second one is still
present.

The attached patch modifies build_user_type_conversion_1 so that
conversion operators are not considered when they should not according
to the standard.

I have successfully regtested it on i386-apple-darwin8.10.1. Is it OK
for the mainline? For 4.2 after 4.2.2 is released?

Best regards,
Simon

:ADDPATCH c++:






[-- Attachment #2: CL_26698 --]
[-- Type: text/plain, Size: 354 bytes --]

2007-10-07  Simon Martin  <simartin@users.sourceforge.net>

	PR c++/26698
	* call.c (build_user_type_conversion_1): Do not consider conversion
	functions to convert a (possibly cv-qualified) object to the (possibly
	cv-qualified) same object type (or a reference to it), to a (possibly
	cv-qualified) base class of that type (or a reference to it).






[-- Attachment #3: pr26698.patch --]
[-- Type: text/plain, Size: 1065 bytes --]

Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 129069)
+++ gcc/cp/call.c	(working copy)
@@ -2601,7 +2601,22 @@ build_user_type_conversion_1 (tree totyp
     ctors = lookup_fnfields (totype, complete_ctor_identifier, 0);
 
   if (IS_AGGR_TYPE (fromtype))
-    conv_fns = lookup_conversions (fromtype);
+    {
+      tree to_main = TYPE_MAIN_VARIANT (non_reference (totype));
+      tree from_main = TYPE_MAIN_VARIANT (fromtype);
+      if (same_type_p (to_main, from_main) ||
+	  (CLASS_TYPE_P (to_main) && CLASS_TYPE_P (from_main)
+	   && DERIVED_FROM_P (to_main, from_main)))
+	{
+	  /* [class.conv.fct] A conversion function is never used to
+	     convert a (possibly cv-qualified) object to the (possibly
+	     cv-qualified) same object type (or a reference to it), to a
+	     (possibly cv-qualified) base class of that type (or a
+	     reference to it)...  */
+	}
+      else
+	conv_fns = lookup_conversions (fromtype);
+    }
 
   candidates = 0;
   flags |= LOOKUP_NO_CONVERSION;






[-- Attachment #4: CL_26698_testsuite --]
[-- Type: text/plain, Size: 117 bytes --]

2007-10-07  Simon Martin  <simartin@users.sourceforge.net>

	PR c++/26698
	* g++.dg/conversion/op4.C: New test.






[-- Attachment #5: op4.C --]
[-- Type: text/plain, Size: 348 bytes --]

/* PR c++/26698 */
/* { dg-do "compile" } */

struct X {
  int x;
  X (int i = 0) : x (i) {}
  operator X& (void) const {
    return *(new X);
  }
};

void add_one (X & ref) { /* { dg-error "in passing argument" } */
  ++ ref.x;
}

void foo() {
  X const a (2);
  add_one(a); /* { dg-error "invalid initialization of reference of type" } */
}

Re: [PATCH] Fix PR c++/26698: g++ accepts const-incorrect code due to conversion function

[Mark Mitchell]

Simon Martin wrote:
> Hi all.

> The attached patch modifies build_user_type_conversion_1 so that
> conversion operators are not considered when they should not according
> to the standard.

This looks good.  Minor note:

> Index: gcc/cp/call.c
> ===================================================================
> --- gcc/cp/call.c	(revision 129069)
> +++ gcc/cp/call.c	(working copy)
> @@ -2601,7 +2601,22 @@ build_user_type_conversion_1 (tree totyp
>      ctors = lookup_fnfields (totype, complete_ctor_identifier, 0);
>  
>    if (IS_AGGR_TYPE (fromtype))
> -    conv_fns = lookup_conversions (fromtype);
> +    {
> +      tree to_main = TYPE_MAIN_VARIANT (non_reference (totype));
> +      tree from_main = TYPE_MAIN_VARIANT (fromtype);
> +      if (same_type_p (to_main, from_main) ||
> +	  (CLASS_TYPE_P (to_main) && CLASS_TYPE_P (from_main)
> +	   && DERIVED_FROM_P (to_main, from_main)))

You can simplify this condition a bit.  If you use
same_type_ignoring_top_level_qualifiers_p, it will automatically do the
TYPE_MAIN_VARIANT bit for you.  And, so will DERIVED_FROM_P, so:

  tree to_non_ref = non_reference (totype);
  if (same_type_ignoring_top_level_qualifiers_p (to_non_ref, fromtype)
      || (CLASS_TYPE_P (to_non_ref) && CLASS_TYPE_P (fromtype)
          && DERIVED_FROM_P (to_non_ref, fromtype))

should work.  OK with that change.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [PATCH] Fix PR c++/26698: g++ accepts const-incorrect code due to conversion function

[Simon Martin]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

Hi Mark.

> You can simplify this condition a bit.  If you use
> same_type_ignoring_top_level_qualifiers_p, it will automatically do the
> TYPE_MAIN_VARIANT bit for you.  And, so will DERIVED_FROM_P, so:
> 
>   tree to_non_ref = non_reference (totype);
>   if (same_type_ignoring_top_level_qualifiers_p (to_non_ref, fromtype)
>       || (CLASS_TYPE_P (to_non_ref) && CLASS_TYPE_P (fromtype)
>           && DERIVED_FROM_P (to_non_ref, fromtype))
> 
> should work.  OK with that change.
It works indeed. I attach the updated patch, that I've successfully
regtested on i386-apple-darwin8.10.1, and that I will commit into trunk
and the 4.2 branch.

Best regards,
Simon





[-- Attachment #2: CL_26698_2 --]
[-- Type: text/plain, Size: 350 bytes --]

2007-10-12  Simon Martin  <simartin@users.sourceforge.net>

	PR c++/26698
	* call.c (build_user_type_conversion_1): Do not consider conversion
	functions to convert a (possibly cv-qualified) object to the (possibly
	cv-qualified) same object type (or a reference to it), to a (possibly
	cv-qualified) base class of that type (or a reference to it).


[-- Attachment #3: pr26698_2.patch --]
[-- Type: text/plain, Size: 1022 bytes --]

Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 129214)
+++ gcc/cp/call.c	(working copy)
@@ -2601,7 +2601,21 @@ build_user_type_conversion_1 (tree totyp
     ctors = lookup_fnfields (totype, complete_ctor_identifier, 0);
 
   if (IS_AGGR_TYPE (fromtype))
-    conv_fns = lookup_conversions (fromtype);
+    {
+      tree to_nonref = non_reference (totype);
+      if (same_type_ignoring_top_level_qualifiers_p (to_nonref, fromtype) ||
+	  (CLASS_TYPE_P (to_nonref) && CLASS_TYPE_P (fromtype)
+	   && DERIVED_FROM_P (to_nonref, fromtype)))
+	{
+	  /* [class.conv.fct] A conversion function is never used to
+	     convert a (possibly cv-qualified) object to the (possibly
+	     cv-qualified) same object type (or a reference to it), to a
+	     (possibly cv-qualified) base class of that type (or a
+	     reference to it)...  */
+	}
+      else
+	conv_fns = lookup_conversions (fromtype);
+    }
 
   candidates = 0;
   flags |= LOOKUP_NO_CONVERSION;