This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PATCH to useless_type_conversion_1 for c++/37016


In 37016 we have a pointer to member function type where one of the argument types is also a pointer to member function type. The C++ front end does not try to canonicalize pmf types, and we end up with two occurrences of this type which need to be structurally compared: one that uses the typedef (the type of the variable) and one that doesn't (the address of the member function). SRA doesn't think that anything will need to be structurally compared except aggregates, so it decides the two METHOD_TYPEs are distinct, and optimizes away the assignment.

The solution is to do structural comparison if TYPE_STRUCTURAL_EQUALITY_P is set on the types.

My first version of this patch limited the comparison for aggregate types to only those with TYPE_STRUCTURAL_EQUALITY_P set as well, but I got some regressions, so the patch I'm checking in doesn't affect the aggregate case.

Tested x86_64-pc-linux-gnu, applied to trunk. Will also apply to 4.3 after testing.
2008-08-04  Jason Merrill  <jason@redhat.com>

	PR c++/37016
	* tree-ssa.c (useless_type_conversion_p_1): Call langhook
	if TYPE_STRUCTURAL_EQUALITY_P is true for both types.

Index: tree-ssa.c
===================================================================
*** tree-ssa.c	(revision 138354)
--- tree-ssa.c	(working copy)
*************** useless_type_conversion_p_1 (tree outer_
*** 1165,1176 ****
        if (TREE_CODE (inner_type) != TREE_CODE (outer_type))
  	return false;
  
!       /* ???  Add structural equivalence check.  */
  
        /* ???  This should eventually just return false.  */
        return lang_hooks.types_compatible_p (inner_type, outer_type);
      }
! 
    return false;
  }
  
--- 1165,1182 ----
        if (TREE_CODE (inner_type) != TREE_CODE (outer_type))
  	return false;
  
!       /* ???  This seems to be necessary even for aggregates that don't
! 	 have TYPE_STRUCTURAL_EQUALITY_P set.  */
  
        /* ???  This should eventually just return false.  */
        return lang_hooks.types_compatible_p (inner_type, outer_type);
      }
!   /* Also for functions and possibly other types with
!      TYPE_STRUCTURAL_EQUALITY_P set.  */
!   else if (TYPE_STRUCTURAL_EQUALITY_P (inner_type)
! 	   && TYPE_STRUCTURAL_EQUALITY_P (outer_type))
!     return lang_hooks.types_compatible_p (inner_type, outer_type);
!   
    return false;
  }
  
Index: testsuite/g++.dg/opt/pmf1.C
===================================================================
*** testsuite/g++.dg/opt/pmf1.C	(revision 0)
--- testsuite/g++.dg/opt/pmf1.C	(revision 0)
***************
*** 0 ****
--- 1,76 ----
+ // PR c++/37016
+ // { dg-do run }
+ // { dg-options "-O2 -Wall" }
+ 
+ /*                                                                              
+   Basic design concept is that WorldObject implements remote method call        
+   functionality using the "curiously recurring template pattern" to enable      
+   forwarding calls from the generic base class that implements the transport    
+   layer to the derived class.                                                   
+ 
+   The specific failure was in forwarding calls to items in a container.         
+   This is emulated here by wrapping a single item.                              
+ 
+   In the main program there are two forms of the call.  In the last             
+   (uncommented) form the member function pointer is for clarity                 
+   assigned to a variable (itemfunptr) before making the call.                   
+   With 4.3.0 and 4.3.1 this code compiles incorrectly with -O1 or greater       
+   to produce this warning                                                       
+ 
+   reproduce.cc: In function â??int main()â??:                                       
+   reproduce.cc:26: warning: â??itemfunptr.void (Container::*)(void
+ (Item::*)(int), int)::__pfnâ?? is used uninitialized in this function             
+   reproduce.cc:47: note: â??itemfunptr.void (Container::*)(void (Item::*)(int),
+ int)::__pfnâ?? was declared here                                                  
+ 
+   and the resulting executable segvs.  It works correctly with -O0.             
+ 
+   With 4.2.3 and earlier it works correctly with optimization.                  
+ 
+   In the first (commented out) form of the call in the main program             
+   we directly refer to desired member function.  This compiles                  
+   and executes correctly with all tested versions.                              
+ */
+ 
+ extern "C" int printf (const char *, ...);
+ 
+ template <class Derived>
+ struct WorldObject {
+     template <typename memfunT, typename arg1T, typename arg2T>
+     void forward(memfunT memfun, arg1T arg1, arg2T arg2) {
+         Derived* obj = static_cast<Derived*>(this);
+         (obj->*memfun)(arg1, arg2);
+     }
+ };
+ 
+ struct Item {
+     void fred(int a) {
+       printf ("a=%d\n", a);
+     }
+ };
+ 
+ struct Container : public WorldObject<Container> {
+     Item item;
+     template <typename itemfunT, typename arg1T>
+     void itemfun(itemfunT itemfun, int a) {
+         (item.*itemfun)(a);
+     }
+ };
+ 
+ int main() {
+     typedef void (Item::*itemfun)(int);
+ 
+     Container t;
+ 
+     // This call compiles and executes correctly with all versions tested       
+     //t.forward(&Container::itemfun<itemfun,int>, &Item::fred, 1);              
+ 
+     // This call compiles with a warning and segvs on execution if using        
+     // -O1 or greater with 4.3.*.  4.2.* is correct.                            
+     void (Container::*itemfunptr)(itemfun, int) =
+ &Container::itemfun<itemfun,int>;
+     t.forward(itemfunptr, &Item::fred, 1);
+ 
+     return 0;
+ }
+ 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]