diff mbox series

[C++,Patch/RFC] PR 83204 ("[6/7/8 Regression] c++ -std=c++14 ICE in maybe_undo_parenthesized_ref, at cp/semantics.c:1694")

Message ID 6789fba4-fd92-587f-bed2-09ae4cb0398c@oracle.com
State New
Headers show
Series [C++,Patch/RFC] PR 83204 ("[6/7/8 Regression] c++ -std=c++14 ICE in maybe_undo_parenthesized_ref, at cp/semantics.c:1694") | expand

Commit Message

Paolo Carlini Feb. 7, 2018, 9:16 p.m. UTC 
Hi,

this is essentially an RFC, I'm still fully analyzing the issue, but 
since I already have something prima facie reasonable and passing the 
testsuite I decided to send immediately out what I have, looking for 
further feedback / help. As fully analyzed by Jakub in the audit trail, 
for the testcase - it's important detail being the parentheses around x 
in the for(int y = 0, yend = (x); y < yend; ++y) part of the lambda body 
- we ICE in maybe_undo_parenthesized_ref when we hit the assert:

       gcc_assert (TREE_CODE (t) == ADDR_EXPR
           || TREE_CODE (t) == STATIC_CAST_EXPR);

because in tsubst_copy_and_build for INDIRECT_REF we do:

-    if (TREE_CODE (r) == INDIRECT_REF)
-      REF_PARENTHESIZED_P (r) = REF_PARENTHESIZED_P (t);
+    if (REF_PARENTHESIZED_P (t))
+      r = force_paren_expr (r);

irrespective of the details of r beyond it being an INDIRECT_REF. Today 
something I immediately noticed, beyond all the ideas provided by Jakub 
in the trail, is that in all the other places in pt.c, besides maybe one 
case in tsubst_qualified_id, where we adjust REF_PARENTHESIZED_P either 
we use indeed force_paren_expr which is guaranteed to produce something 
consistent with maybe_undo_parenthesized_ref, or we simply do things like:

         if (TREE_CODE (r) == COMPONENT_REF)
           REF_PARENTHESIZED_P (r) = REF_PARENTHESIZED_P (t);

where in fact force_paren_expr would simply handle a COMPONENT_REF in 
the same way. Thus the idea of the draft patch. Jason, if you see 
something deeper going on which you would rather prefer to handle 
yourself, please let me know and I'll adjust the bug owner appropriately.

Thanks! Paolo.

////////////////////

Comments

Jason Merrill Feb. 7, 2018, 9:25 p.m. UTC | #1 
OK.

On Wed, Feb 7, 2018 at 4:16 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> this is essentially an RFC, I'm still fully analyzing the issue, but since I
> already have something prima facie reasonable and passing the testsuite I
> decided to send immediately out what I have, looking for further feedback /
> help. As fully analyzed by Jakub in the audit trail, for the testcase - it's
> important detail being the parentheses around x in the for(int y = 0, yend =
> (x); y < yend; ++y) part of the lambda body - we ICE in
> maybe_undo_parenthesized_ref when we hit the assert:
>
>       gcc_assert (TREE_CODE (t) == ADDR_EXPR
>           || TREE_CODE (t) == STATIC_CAST_EXPR);
>
> because in tsubst_copy_and_build for INDIRECT_REF we do:
>
> -    if (TREE_CODE (r) == INDIRECT_REF)
> -      REF_PARENTHESIZED_P (r) = REF_PARENTHESIZED_P (t);
> +    if (REF_PARENTHESIZED_P (t))
> +      r = force_paren_expr (r);
>
> irrespective of the details of r beyond it being an INDIRECT_REF. Today
> something I immediately noticed, beyond all the ideas provided by Jakub in
> the trail, is that in all the other places in pt.c, besides maybe one case
> in tsubst_qualified_id, where we adjust REF_PARENTHESIZED_P either we use
> indeed force_paren_expr which is guaranteed to produce something consistent
> with maybe_undo_parenthesized_ref, or we simply do things like:
>
>         if (TREE_CODE (r) == COMPONENT_REF)
>           REF_PARENTHESIZED_P (r) = REF_PARENTHESIZED_P (t);
>
> where in fact force_paren_expr would simply handle a COMPONENT_REF in the
> same way. Thus the idea of the draft patch. Jason, if you see something
> deeper going on which you would rather prefer to handle yourself, please let
> me know and I'll adjust the bug owner appropriately.
>
> Thanks! Paolo.
>
> ////////////////////
>
diff mbox series

Patch

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 257458)
+++ cp/pt.c	(working copy)
@@ -17224,8 +17224,8 @@  tsubst_copy_and_build (tree t,
 	  r = build_x_indirect_ref (input_location, r, RO_UNARY_STAR,
 				    complain|decltype_flag);
 
-	if (TREE_CODE (r) == INDIRECT_REF)
-	  REF_PARENTHESIZED_P (r) = REF_PARENTHESIZED_P (t);
+	if (REF_PARENTHESIZED_P (t))
+	  r = force_paren_expr (r);
 
 	RETURN (r);
       }
Index: testsuite/g++.dg/cpp0x/lambda/lambda-ice25.C
===================================================================
--- testsuite/g++.dg/cpp0x/lambda/lambda-ice25.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/lambda/lambda-ice25.C	(working copy)
@@ -0,0 +1,27 @@ 
+// PR c++/83204
+// { dg-do compile { target c++11 } }
+
+int rand();
+
+template<typename T>
+struct s
+{
+    int count() { return rand(); }
+};
+
+template<typename v>
+void f(s<v> a)
+{
+    int const x = a.count();
+    int r = 0;
+    auto l = [&](int& r)
+    {
+        for(int y = 0, yend = (x); y < yend; ++y)
+        {
+            r += y;
+        }
+    };
+    l(r);
+}
+
+template void f(s<float>);