[C++ Patch] for c++/11750

[C++ Patch] for c++/11750

[Fabien Chêne]

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

Hi,

Here, we were setting the LOOKUP_NONVIRTUAL flag wrongly. Actually, we
need to check if the function context is the same than the instance
type -- yes that might happen that they be different in presence of
using-declarations.

It happens that it was working if the call was invoked through a
pointer, that's because  we were failing to determine the dynamic type
(in resolved_fixed_type_p). On the contrary, it wasn't working if the
call was done through a reference because we manage to determine the
dynamic_type thanks to a special case in fixed_type_or_null. There is
probably room for improvement here, though I'm not sure the C++ front
end is the better place to de-virtualize.

Tested x84_64-unknown-linux-gnu without regressions. OK to commit ?

gcc/testsuite/ChangeLog

2012-08-12  Fabien Chêne  <fabien@gcc.gnu.org>

	PR c++/11750
	* g++.dg/inherit/vitual9.C: New.

gcc/cp/ChangeLog

2012-08-12  Fabien Chêne  <fabien@gcc.gnu.org>

	PR c++/11750
	* call.c (build_new_method_call_1): Check that the instance type
	and the function context are the same before setting the flag
	LOOKUP_NONVIRTUAL.


-- 
Fabien

[-- Attachment #2: pr11750.patch --]
[-- Type: application/octet-stream, Size: 1699 bytes --]

Index: cp/call.c
===================================================================
--- cp/call.c	(revision 188293)
+++ cp/call.c	(working copy)
@@ -7449,9 +7449,14 @@
 	    }
 	  else
 	    {
-	      /* Optimize away vtable lookup if we know that this function
-		 can't be overridden.  */
+	      /* Optimize away vtable lookup if we know that this
+		 function can't be overridden.  We need to check if
+		 the context and the instance type are the same,
+		 actually FN migth be defined in a different class
+		 type because of a using-declaration. In this case, we
+		 do not want to perform a non-virtual call.  */
 	      if (DECL_VINDEX (fn) && ! (flags & LOOKUP_NONVIRTUAL)
+		  && same_type_p (DECL_CONTEXT (fn), TREE_TYPE (instance))
 		  && resolves_to_fixed_type_p (instance, 0))
 		flags |= LOOKUP_NONVIRTUAL;
               if (explicit_targs)
Index: testsuite/g++.dg/inherit/virtual9.C
===================================================================
--- testsuite/g++.dg/inherit/virtual9.C	(revision 0)
+++ testsuite/g++.dg/inherit/virtual9.C	(revision 0)
@@ -0,0 +1,44 @@
+// { dg-do run }
+// PR c++/11750
+
+struct A
+{
+  virtual void f() const { __builtin_abort(); }
+  virtual void g() {}
+};
+
+struct B : virtual A
+{
+  virtual void f() const {}
+  virtual void g() { __builtin_abort(); }
+};
+
+struct C : B, virtual A
+{
+  using A::f;
+  using A::g;
+};
+
+int main()
+{
+  C c;
+  c.f(); // call B::f
+
+  C c2;
+  c2.C::g(); // call A::g
+
+  C* c3 = &c;
+  c3->f(); // call B::f
+
+  C& c4 = c;
+  c4.f(); // call B::f
+
+  C const* c5 = &c;
+  c5->f(); // call B::f
+
+  C** c6 = &c3;
+  (*c6)->f(); // call B::f
+
+  C const& c7 = c;
+  c7.f(); // call B::f
+}

Re: [C++ Patch] for c++/11750

[Fabien Chêne]

Jason, could you please have a look at this (rather old) one ?
Thanks.

2012/8/13 Fabien Chêne <fabien.chene@gmail.com>:
> Hi,
>
> Here, we were setting the LOOKUP_NONVIRTUAL flag wrongly. Actually, we
> need to check if the function context is the same than the instance
> type -- yes that might happen that they be different in presence of
> using-declarations.
>
> It happens that it was working if the call was invoked through a
> pointer, that's because  we were failing to determine the dynamic type
> (in resolved_fixed_type_p). On the contrary, it wasn't working if the
> call was done through a reference because we manage to determine the
> dynamic_type thanks to a special case in fixed_type_or_null. There is
> probably room for improvement here, though I'm not sure the C++ front
> end is the better place to de-virtualize.
>
> Tested x84_64-unknown-linux-gnu without regressions. OK to commit ?
>
> gcc/testsuite/ChangeLog
>
> 2012-08-12  Fabien Chêne  <fabien@gcc.gnu.org>
>
>         PR c++/11750
>         * g++.dg/inherit/vitual9.C: New.
>
> gcc/cp/ChangeLog
>
> 2012-08-12  Fabien Chêne  <fabien@gcc.gnu.org>
>
>         PR c++/11750
>         * call.c (build_new_method_call_1): Check that the instance type
>         and the function context are the same before setting the flag
>         LOOKUP_NONVIRTUAL.
>
>
> --
> Fabien

-- 
Fabien

Re: [C++ Patch] for c++/11750

[Jason Merrill]

On 08/13/2012 04:49 PM, Fabien Chêne wrote:
> +	      /* Optimize away vtable lookup if we know that this
> +		 function can't be overridden.  We need to check if
> +		 the context and the instance type are the same,
> +		 actually FN migth be defined in a different class

Typo: "might"

> +		 type because of a using-declaration. In this case, we
> +		 do not want to perform a non-virtual call.  */
>   	      if (DECL_VINDEX (fn) && ! (flags & LOOKUP_NONVIRTUAL)
> +		  && same_type_p (DECL_CONTEXT (fn), TREE_TYPE (instance))
>   		  && resolves_to_fixed_type_p (instance, 0))

This should be same_type_ignoring_top_level_qualifiers_p.

OK with that change.

Jason