public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ Patch] Fix c++/56243
@ 2013-02-25 21:10 Fabien Chêne
  2013-02-25 23:24 ` Jason Merrill
  0 siblings, 1 reply; 5+ messages in thread
From: Fabien Chêne @ 2013-02-25 21:10 UTC (permalink / raw)
  To: Jason Merrill, GCC Patches

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

Hi,

Please see details on Bugzilla.
Tested x86_64-unknown-linux-gnu without regressions.

gcc/testsuite/ChangeLog

2012-02-25  Fabien Chêne  <fabien@gcc.gnu.org>

	PR c++/56243
	* g++.dg/cpp0x/pr56243.C: New.

gcc/cp/ChangeLog

2012-02-25  Fabien Chêne  <fabien@gcc.gnu.org>

	PR c++/56243
	* class.c (fixed_type_or_null): Check that the first operand of a
	COMPONENT_REF is actually a FIELD_DECL before calling
	DECL_FIELD_IS_BASE on it.

-- 
Fabien

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

Index: gcc/testsuite/g++.dg/cpp0x/pr56243.C
===================================================================
--- gcc/testsuite/g++.dg/cpp0x/pr56243.C	(revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/pr56243.C	(revision 0)
@@ -0,0 +1,22 @@
+// PR c++/56243 
+
+struct A
+{
+    virtual int String ();
+};
+
+struct F : A { };
+
+struct G
+{
+    F value;
+};
+
+class D
+{
+    template < int N > void Verify() {
+      G<F> x;
+      F& name = x.value;
+      name.String();
+    }
+};
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 196018)
+++ gcc/cp/class.c	(working copy)
@@ -6642,9 +6642,12 @@ fixed_type_or_null (tree instance, int *
     case COMPONENT_REF:
       /* If this component is really a base class reference, then the field
 	 itself isn't definitive.  */
-      if (DECL_FIELD_IS_BASE (TREE_OPERAND (instance, 1)))
+      {
+	tree field = TREE_OPERAND (instance, 1);
+	if (TREE_CODE (field) == FIELD_DECL && DECL_FIELD_IS_BASE (field))
 	return RECUR (TREE_OPERAND (instance, 0));
-      return RECUR (TREE_OPERAND (instance, 1));
+      return RECUR (field);
+      }
 
     case VAR_DECL:
     case FIELD_DECL:

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [C++ Patch] Fix c++/56243
  2013-02-25 21:10 [C++ Patch] Fix c++/56243 Fabien Chêne
@ 2013-02-25 23:24 ` Jason Merrill
  2013-02-25 23:25   ` Jason Merrill
  0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2013-02-25 23:24 UTC (permalink / raw)
  To: Fabien Chêne, GCC Patches

I think my preference would be to avoid calling fixed_type_or_null at 
all when we're in a template.  I already changed 
resolves_to_fixed_type_p that way, now we need to fix build_vtbl_ref_1. 
  For post 4.8 we might avoid messing with vtables at all when we're in 
a template.

Jason

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [C++ Patch] Fix c++/56243
  2013-02-25 23:24 ` Jason Merrill
@ 2013-02-25 23:25   ` Jason Merrill
  2013-02-26 15:21     ` Jason Merrill
  2013-02-28 15:58     ` Jason Merrill
  0 siblings, 2 replies; 5+ messages in thread
From: Jason Merrill @ 2013-02-25 23:25 UTC (permalink / raw)
  To: Fabien Chêne, GCC Patches

On 02/25/2013 06:24 PM, Jason Merrill wrote:
> I think my preference would be to avoid calling fixed_type_or_null at
> all when we're in a template.  I already changed
> resolves_to_fixed_type_p that way, now we need to fix build_vtbl_ref_1.

Oops, now I see the discussion on the PR.  I'll take a look.

Jason


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [C++ Patch] Fix c++/56243
  2013-02-25 23:25   ` Jason Merrill
@ 2013-02-26 15:21     ` Jason Merrill
  2013-02-28 15:58     ` Jason Merrill
  1 sibling, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2013-02-26 15:21 UTC (permalink / raw)
  To: Fabien Chêne, GCC Patches

On 02/25/2013 06:25 PM, Jason Merrill wrote:
> On 02/25/2013 06:24 PM, Jason Merrill wrote:
>> I think my preference would be to avoid calling fixed_type_or_null at
>> all when we're in a template.  I already changed
>> resolves_to_fixed_type_p that way, now we need to fix build_vtbl_ref_1.
>
> Oops, now I see the discussion on the PR.  I'll take a look.

I can't reproduce the regression on constexpr-static10.C.  What was the 
earlier patch that was having trouble?

Jason


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [C++ Patch] Fix c++/56243
  2013-02-25 23:25   ` Jason Merrill
  2013-02-26 15:21     ` Jason Merrill
@ 2013-02-28 15:58     ` Jason Merrill
  1 sibling, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2013-02-28 15:58 UTC (permalink / raw)
  To: Fabien Chêne, GCC Patches

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

On 02/25/2013 06:25 PM, Jason Merrill wrote:
> On 02/25/2013 06:24 PM, Jason Merrill wrote:
>> I think my preference would be to avoid calling fixed_type_or_null at
>> all when we're in a template.  I already changed
>> resolves_to_fixed_type_p that way, now we need to fix build_vtbl_ref_1.
>
> Oops, now I see the discussion on the PR.  I'll take a look.

I'm applying this patch.  When we're in fold_non_dependent_expr we care 
about doing the right thing for possible constant-expressions, but a 
virtual function call can't be part of a constant-expression, so we 
don't need to worry about virtual lookup.

Tested x86_64-pc-linux-gnu, applying to trunk.


[-- Attachment #2: 56243.patch --]
[-- Type: text/x-patch, Size: 1338 bytes --]

commit 8d561a71fbe141ad4d5b4f1ff9160e4f4c81a061
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Feb 26 10:06:31 2013 -0500

    	PR c++/56243
    	* call.c (build_over_call): Avoid virtual lookup in a template.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 7c41421..4eb38ec 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7033,7 +7033,10 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
   if (!already_used)
     mark_used (fn);
 
-  if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0)
+  if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0
+      /* Don't mess with virtual lookup in fold_non_dependent_expr; virtual
+	 functions can't be constexpr.  */
+      && !in_template_function ())
     {
       tree t;
       tree binfo = lookup_base (TREE_TYPE (TREE_TYPE (argarray[0])),
diff --git a/gcc/testsuite/g++.dg/template/virtual4.C b/gcc/testsuite/g++.dg/template/virtual4.C
new file mode 100644
index 0000000..a2c7420b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/virtual4.C
@@ -0,0 +1,30 @@
+// PR c++/56243
+
+struct A
+{
+  virtual int String ();
+};
+
+struct F: A { };
+
+struct G
+{
+  F value;
+};
+
+struct D
+{
+  template <int>
+  void Verify()
+  {
+    G x;
+    F& name = x.value;
+    name.String();
+  }
+};
+
+int main()
+{
+  D d;
+  d.Verify<42>();
+}

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-02-28 15:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-25 21:10 [C++ Patch] Fix c++/56243 Fabien Chêne
2013-02-25 23:24 ` Jason Merrill
2013-02-25 23:25   ` Jason Merrill
2013-02-26 15:21     ` Jason Merrill
2013-02-28 15:58     ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).