Message ID | 20181012005601.GK19003@redhat.com |
---|---|
State | New |
Headers | show |
Series | C++ PATCH for c++/87594, constexpr rejects-valid with range-based for | expand |
On 10/11/18 8:56 PM, Marek Polacek wrote: > Here potential_constant_expression_1 rejects the testcase because the body of > the for loop calls a non-constexpr function. But the range is empty so the > function would never get called. > The trick with evaluating the for-condition doesn't work here, because we're > dealing with a converted range-based for and can't evaluate > > __for_begin != __for_end > > because the constexpr cache doesn't have the values of these two VAR_DECLs. > So either we can use this ugly hack (more specialized), or just not check the > body of the loop (more general), similarly to what we do (don't do) with > switch. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-10-11 Marek Polacek <polacek@redhat.com> > > PR c++/87594 - constexpr rejects-valid with range-based for. > * constexpr.c (potential_constant_expression_1) <case FOR_STMT>: Return > true for a converted ange-based for-statement. > > * g++.dg/cpp1y/constexpr-loop8.C: New test. > > diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c > index 4fa8c965a9d..685ca743859 100644 > --- gcc/cp/constexpr.c > +++ gcc/cp/constexpr.c > @@ -5827,6 +5827,17 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, > tmp = cxx_eval_outermost_constant_expr (tmp, true); > if (integer_zerop (tmp)) > return true; > + /* See if this is > + __for_begin != __for_end > + cp_convert_range_for created for us. If so, this is a converted > + range-based for-statement, and we're not able to evaluate this > + condition, so we might end up skipping the body entirely. */ > + else if (TREE_CODE (tmp) == NE_EXPR > + && VAR_P (TREE_OPERAND (tmp, 0)) > + && DECL_NAME (TREE_OPERAND (tmp, 0)) == for_begin_identifier > + && VAR_P (TREE_OPERAND (tmp, 1)) > + && DECL_NAME (TREE_OPERAND (tmp, 1)) == for_end_identifier) > + return true; This seems over-specific; any other condition we can't immediately evaluate also might not ever be true. So I think we want to return here unless the condition is always true. Likewise for WHILE_STMT. Jason
On Wed, Oct 24, 2018 at 05:16:33PM -0400, Jason Merrill wrote: > On 10/11/18 8:56 PM, Marek Polacek wrote: > > Here potential_constant_expression_1 rejects the testcase because the body of > > the for loop calls a non-constexpr function. But the range is empty so the > > function would never get called. > > The trick with evaluating the for-condition doesn't work here, because we're > > dealing with a converted range-based for and can't evaluate > > > > __for_begin != __for_end > > > > because the constexpr cache doesn't have the values of these two VAR_DECLs. > > > > So either we can use this ugly hack (more specialized), or just not check the > > body of the loop (more general), similarly to what we do (don't do) with > > switch. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > 2018-10-11 Marek Polacek <polacek@redhat.com> > > > > PR c++/87594 - constexpr rejects-valid with range-based for. > > * constexpr.c (potential_constant_expression_1) <case FOR_STMT>: Return > > true for a converted ange-based for-statement. > > > > * g++.dg/cpp1y/constexpr-loop8.C: New test. > > > > diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c > > index 4fa8c965a9d..685ca743859 100644 > > --- gcc/cp/constexpr.c > > +++ gcc/cp/constexpr.c > > @@ -5827,6 +5827,17 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, > > tmp = cxx_eval_outermost_constant_expr (tmp, true); > > if (integer_zerop (tmp)) > > return true; > > + /* See if this is > > + __for_begin != __for_end > > + cp_convert_range_for created for us. If so, this is a converted > > + range-based for-statement, and we're not able to evaluate this > > + condition, so we might end up skipping the body entirely. */ > > + else if (TREE_CODE (tmp) == NE_EXPR > > + && VAR_P (TREE_OPERAND (tmp, 0)) > > + && DECL_NAME (TREE_OPERAND (tmp, 0)) == for_begin_identifier > > + && VAR_P (TREE_OPERAND (tmp, 1)) > > + && DECL_NAME (TREE_OPERAND (tmp, 1)) == for_end_identifier) > > + return true; > > This seems over-specific; any other condition we can't immediately evaluate > also might not ever be true. So I think we want to return here unless the > condition is always true. Likewise for WHILE_STMT. Yeah, that makes a lot of sense. Thus: Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-10-29 Marek Polacek <polacek@redhat.com> PR c++/87594 - constexpr rejects-valid with range-based for. * constexpr.c (potential_constant_expression_1): If the condition can't be evaluated, return true. * g++.dg/cpp1y/constexpr-loop8.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 4fa8c965a9d..7692b1727da 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -5825,7 +5825,9 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, { if (!processing_template_decl) tmp = cxx_eval_outermost_constant_expr (tmp, true); - if (integer_zerop (tmp)) + /* If we couldn't evaluate the condition, it might not ever be + true. */ + if (!integer_onep (tmp)) return true; } if (!RECUR (FOR_EXPR (t), any)) @@ -5853,7 +5855,8 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, return false; if (!processing_template_decl) tmp = cxx_eval_outermost_constant_expr (tmp, true); - if (integer_zerop (tmp)) + /* If we couldn't evaluate the condition, it might not ever be true. */ + if (!integer_onep (tmp)) return true; if (!RECUR (WHILE_BODY (t), any)) return false; diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-loop8.C gcc/testsuite/g++.dg/cpp1y/constexpr-loop8.C index e69de29bb2d..bf132f2484e 100644 --- gcc/testsuite/g++.dg/cpp1y/constexpr-loop8.C +++ gcc/testsuite/g++.dg/cpp1y/constexpr-loop8.C @@ -0,0 +1,44 @@ +// PR c++/87594 +// { dg-do compile { target c++14 } } + +constexpr bool always_false() { return false; } +int f() { return 1; } + +constexpr int +fn1() +{ + struct empty_range { + constexpr int* begin() { return 0; } + constexpr int* end() { return 0; } + } e; + for (auto x : e) + f(); + return 0; +} + +constexpr int +fn2 () +{ + int a[] = { 1, 2, 3 }; + for (auto x : a) + f(); // { dg-error "call to non-.constexpr. function" } + return 0; +} + +constexpr int +fn3 () +{ + __extension__ int a[] = { }; + for (auto x : a) + f(); + return 0; +} + + +void +bar () +{ + constexpr int i1 = fn1 (); + constexpr int i2 = fn2 (); // { dg-message "in .constexpr. expansion of " } + constexpr int i3 = fn3 (); +}
On 10/29/18 2:07 PM, Marek Polacek wrote: > On Wed, Oct 24, 2018 at 05:16:33PM -0400, Jason Merrill wrote: >> On 10/11/18 8:56 PM, Marek Polacek wrote: >>> Here potential_constant_expression_1 rejects the testcase because the body of >>> the for loop calls a non-constexpr function. But the range is empty so the >>> function would never get called. >>> The trick with evaluating the for-condition doesn't work here, because we're >>> dealing with a converted range-based for and can't evaluate >>> >>> __for_begin != __for_end >>> >>> because the constexpr cache doesn't have the values of these two VAR_DECLs. >> >> >>> So either we can use this ugly hack (more specialized), or just not check the >>> body of the loop (more general), similarly to what we do (don't do) with >>> switch. >>> >>> Bootstrapped/regtested on x86_64-linux, ok for trunk? >>> >>> 2018-10-11 Marek Polacek <polacek@redhat.com> >>> >>> PR c++/87594 - constexpr rejects-valid with range-based for. >>> * constexpr.c (potential_constant_expression_1) <case FOR_STMT>: Return >>> true for a converted ange-based for-statement. >>> >>> * g++.dg/cpp1y/constexpr-loop8.C: New test. >>> >>> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c >>> index 4fa8c965a9d..685ca743859 100644 >>> --- gcc/cp/constexpr.c >>> +++ gcc/cp/constexpr.c >>> @@ -5827,6 +5827,17 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, >>> tmp = cxx_eval_outermost_constant_expr (tmp, true); >>> if (integer_zerop (tmp)) >>> return true; >>> + /* See if this is >>> + __for_begin != __for_end >>> + cp_convert_range_for created for us. If so, this is a converted >>> + range-based for-statement, and we're not able to evaluate this >>> + condition, so we might end up skipping the body entirely. */ >>> + else if (TREE_CODE (tmp) == NE_EXPR >>> + && VAR_P (TREE_OPERAND (tmp, 0)) >>> + && DECL_NAME (TREE_OPERAND (tmp, 0)) == for_begin_identifier >>> + && VAR_P (TREE_OPERAND (tmp, 1)) >>> + && DECL_NAME (TREE_OPERAND (tmp, 1)) == for_end_identifier) >>> + return true; >> >> This seems over-specific; any other condition we can't immediately evaluate >> also might not ever be true. So I think we want to return here unless the >> condition is always true. Likewise for WHILE_STMT. > > Yeah, that makes a lot of sense. Thus: > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-10-29 Marek Polacek <polacek@redhat.com> > > PR c++/87594 - constexpr rejects-valid with range-based for. > * constexpr.c (potential_constant_expression_1): If the condition > can't be evaluated, return true. OK. Jason
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 4fa8c965a9d..685ca743859 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -5827,6 +5827,17 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, tmp = cxx_eval_outermost_constant_expr (tmp, true); if (integer_zerop (tmp)) return true; + /* See if this is + __for_begin != __for_end + cp_convert_range_for created for us. If so, this is a converted + range-based for-statement, and we're not able to evaluate this + condition, so we might end up skipping the body entirely. */ + else if (TREE_CODE (tmp) == NE_EXPR + && VAR_P (TREE_OPERAND (tmp, 0)) + && DECL_NAME (TREE_OPERAND (tmp, 0)) == for_begin_identifier + && VAR_P (TREE_OPERAND (tmp, 1)) + && DECL_NAME (TREE_OPERAND (tmp, 1)) == for_end_identifier) + return true; } if (!RECUR (FOR_EXPR (t), any)) return false; diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-loop8.C gcc/testsuite/g++.dg/cpp1y/constexpr-loop8.C index e69de29bb2d..bf132f2484e 100644 --- gcc/testsuite/g++.dg/cpp1y/constexpr-loop8.C +++ gcc/testsuite/g++.dg/cpp1y/constexpr-loop8.C @@ -0,0 +1,44 @@ +// PR c++/87594 +// { dg-do compile { target c++14 } } + +constexpr bool always_false() { return false; } +int f() { return 1; } + +constexpr int +fn1() +{ + struct empty_range { + constexpr int* begin() { return 0; } + constexpr int* end() { return 0; } + } e; + for (auto x : e) + f(); + return 0; +} + +constexpr int +fn2 () +{ + int a[] = { 1, 2, 3 }; + for (auto x : a) + f(); // { dg-error "call to non-.constexpr. function" } + return 0; +} + +constexpr int +fn3 () +{ + __extension__ int a[] = { }; + for (auto x : a) + f(); + return 0; +} + + +void +bar () +{ + constexpr int i1 = fn1 (); + constexpr int i2 = fn2 (); // { dg-message "in .constexpr. expansion of " } + constexpr int i3 = fn3 (); +}