[C++] Fix PR82357 - bogus "cannot bind bitfield" error

Message ID 20171013094053.GC235@x4
State New
Headers show
Series
  • [C++] Fix PR82357 - bogus "cannot bind bitfield" error
Related show

Commit Message

Markus Trippelsdorf Oct. 13, 2017, 9:40 a.m.
r253266 introduced a bogus "cannot bind bitfield" error that breaks
building Chromium and Node.js.
Fix by removing the ugly goto.

Tested on ppc64le.
Ok for trunk?
Thanks.

	PR c++/82357
	* typeck.c (build_static_cast): Handle processing_template_decl
	without using goto.

Comments

Jason Merrill Oct. 13, 2017, 4:02 p.m. | #1
On Fri, Oct 13, 2017 at 5:40 AM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> r253266 introduced a bogus "cannot bind bitfield" error that breaks
> building Chromium and Node.js.
> Fix by removing the ugly goto.
>
> Tested on ppc64le.
> Ok for trunk?

No, this just undoes my change, so we go back to not doing type
checking for non-dependent static casts.  Better I think to avoid the
call to build_static_cast in the first place, by teaching
stabilize_reference that it can return a NON_DEPENDENT_EXPR unchanged.
How does this (untested) patch work for you?
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index e21ff6a1572..366f46f1506 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -333,6 +333,10 @@ cp_stabilize_reference (tree ref)
 {
   switch (TREE_CODE (ref))
     {
+    case NON_DEPENDENT_EXPR:
+      /* We aren't actually evaluating this.  */
+      return ref;
+
     /* We need to treat specially anything stabilize_reference doesn't
        handle specifically.  */
     case VAR_DECL:
Markus Trippelsdorf Oct. 13, 2017, 5:21 p.m. | #2
On 2017.10.13 at 12:02 -0400, Jason Merrill wrote:
> On Fri, Oct 13, 2017 at 5:40 AM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
> > r253266 introduced a bogus "cannot bind bitfield" error that breaks
> > building Chromium and Node.js.
> > Fix by removing the ugly goto.
> >
> > Tested on ppc64le.
> > Ok for trunk?
> 
> No, this just undoes my change, so we go back to not doing type
> checking for non-dependent static casts.  Better I think to avoid the
> call to build_static_cast in the first place, by teaching
> stabilize_reference that it can return a NON_DEPENDENT_EXPR unchanged.
> How does this (untested) patch work for you?

It works fine. Thanks.
It would be good to have a testcase that checks the type checking for
non-dependent static casts.
I'll let you handle the current issue.
Jason Merrill Oct. 13, 2017, 7:04 p.m. | #3
On Fri, Oct 13, 2017 at 1:21 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2017.10.13 at 12:02 -0400, Jason Merrill wrote:
>> On Fri, Oct 13, 2017 at 5:40 AM, Markus Trippelsdorf
>> <markus@trippelsdorf.de> wrote:
>> > r253266 introduced a bogus "cannot bind bitfield" error that breaks
>> > building Chromium and Node.js.
>> > Fix by removing the ugly goto.
>> >
>> > Tested on ppc64le.
>> > Ok for trunk?
>>
>> No, this just undoes my change, so we go back to not doing type
>> checking for non-dependent static casts.  Better I think to avoid the
>> call to build_static_cast in the first place, by teaching
>> stabilize_reference that it can return a NON_DEPENDENT_EXPR unchanged.
>> How does this (untested) patch work for you?
>
> It works fine. Thanks.
> It would be good to have a testcase that checks the type checking for
> non-dependent static casts.
> I'll let you handle the current issue.

Done.
commit d570081ae5d9afd421acccb10eef82baab2b35da
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Oct 13 12:31:21 2017 -0400

            PR c++/82357 - bit-field in template
    
            * tree.c (cp_stabilize_reference): Just return a NON_DEPENDENT_EXPR.

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index e21ff6a1572..366f46f1506 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -333,6 +333,10 @@ cp_stabilize_reference (tree ref)
 {
   switch (TREE_CODE (ref))
     {
+    case NON_DEPENDENT_EXPR:
+      /* We aren't actually evaluating this.  */
+      return ref;
+
     /* We need to treat specially anything stabilize_reference doesn't
        handle specifically.  */
     case VAR_DECL:
diff --git a/gcc/testsuite/g++.dg/template/bitfield4.C b/gcc/testsuite/g++.dg/template/bitfield4.C
new file mode 100644
index 00000000000..4927b7ab144
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/bitfield4.C
@@ -0,0 +1,6 @@
+// PR c++/82357
+
+template <typename> struct A {
+  A() { x |= 0; }
+  int x : 8;
+};
diff --git a/gcc/testsuite/g++.dg/template/cast4.C b/gcc/testsuite/g++.dg/template/cast4.C
new file mode 100644
index 00000000000..2f46c7189eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/cast4.C
@@ -0,0 +1,4 @@
+template <class T> void f()
+{
+  static_cast<int&>(42);	// { dg-error "static_cast" }
+}

Patch

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 08b2ae555e63..00688a21421c 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -7059,9 +7059,8 @@  build_static_cast (tree type, tree oexpr, tsubst_flags_t complain)
 
   bool dependent = (dependent_type_p (type)
 		    || type_dependent_expression_p (expr));
-  if (dependent)
+  if (dependent || processing_template_decl)
     {
-    tmpl:
       expr = build_min (STATIC_CAST_EXPR, type, oexpr);
       /* We don't know if it will or will not have side effects.  */
       TREE_SIDE_EFFECTS (expr) = 1;
@@ -7084,8 +7083,6 @@  build_static_cast (tree type, tree oexpr, tsubst_flags_t complain)
 	  maybe_warn_about_useless_cast (type, expr, complain);
 	  maybe_warn_about_cast_ignoring_quals (type, complain);
 	}
-      if (processing_template_decl)
-	goto tmpl;
       return result;
     }
 
diff --git a/gcc/testsuite/g++.dg/template/bitfield4.C b/gcc/testsuite/g++.dg/template/bitfield4.C
new file mode 100644
index 000000000000..d53b0d406275
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/bitfield4.C
@@ -0,0 +1,6 @@ 
+// PR c++/82357
+
+template <typename> struct A {
+  A() { x |= 0; } // { dg-bogus "cannot bind bitfield" }
+  int x : 8;
+};