| Message ID | CAEwic4a+WSZouoT8kP9DVKYTkS4xAoseA-F5Fvod6D_JapiJrQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
2015-08-31 22:19 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: > 2015-08-31 21:43 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >> 2015-08-31 21:29 GMT+02:00 Jason Merrill <jason@redhat.com>: >>> On 08/31/2015 03:08 PM, Kai Tietz wrote: >>>> >>>> I will need to verify that this patch doesn't introduce regressions. >>>> The wacky thing here is the encapsulation of overflowed-arguments in >>>> maybe_constant_value function by nop-expr. >>> >>> >>> Do we need to worry about that? If one of the operands is overflowed, we >>> don't care whether the result is overflowed. >> >> Well, we would introduce, if we don't see in condition that operand >> already overflowed, double overflow-warning, which seems to be >> something we avoided until now. So I would say, it matters. >> >> Kai > > Similar to the binary-operation we want to do then the same for > unary-operations, too. > > Eg. testcase: > > #include <limits.h> > > constexpr int f() { return INT_MIN; } > > int main() > { > return -f(); // { dg-warning "overflow" } > } > With following patch we do diagnostics for it. > > Kai > > Index: semantics.c > =================================================================== > --- semantics.c (Revision 227339) > +++ semantics.c (Arbeitskopie) > @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co > tree result = build_x_unary_op (loc, code, expr, complain); > tree result_ovl = result; > > - expr_ovl = fold_simple (expr_ovl); > - result_ovl = fold_simple (result); > - > + expr_ovl = maybe_constant_value (expr_ovl); > + result_ovl = maybe_constant_value (result); > + /* Strip nop-expressions added by maybe_constant_value on overflow. */ > + STRIP_NOPS (expr_ovl); > + STRIP_NOPS (result_ovl); > if ((complain & tf_warning) > && TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl)) > overflow_warning (input_location, result_ovl); I committed patches for binary & unary operations together with testcases. Regression-run still running. There seems to be additional expressions needed in constexpr for this. For now we have a bootstrap-issue due cast_expr in cxx_eval_constant_expression. There might be more of them ... Kai
2015-09-01 10:15 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: > 2015-08-31 22:19 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >> 2015-08-31 21:43 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >>> 2015-08-31 21:29 GMT+02:00 Jason Merrill <jason@redhat.com>: >>>> On 08/31/2015 03:08 PM, Kai Tietz wrote: >>>>> >>>>> I will need to verify that this patch doesn't introduce regressions. >>>>> The wacky thing here is the encapsulation of overflowed-arguments in >>>>> maybe_constant_value function by nop-expr. >>>> >>>> >>>> Do we need to worry about that? If one of the operands is overflowed, we >>>> don't care whether the result is overflowed. >>> >>> Well, we would introduce, if we don't see in condition that operand >>> already overflowed, double overflow-warning, which seems to be >>> something we avoided until now. So I would say, it matters. >>> >>> Kai >> >> Similar to the binary-operation we want to do then the same for >> unary-operations, too. >> >> Eg. testcase: >> >> #include <limits.h> >> >> constexpr int f() { return INT_MIN; } >> >> int main() >> { >> return -f(); // { dg-warning "overflow" } >> } >> With following patch we do diagnostics for it. >> >> Kai >> >> Index: semantics.c >> =================================================================== >> --- semantics.c (Revision 227339) >> +++ semantics.c (Arbeitskopie) >> @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co >> tree result = build_x_unary_op (loc, code, expr, complain); >> tree result_ovl = result; >> >> - expr_ovl = fold_simple (expr_ovl); >> - result_ovl = fold_simple (result); >> - >> + expr_ovl = maybe_constant_value (expr_ovl); >> + result_ovl = maybe_constant_value (result); >> + /* Strip nop-expressions added by maybe_constant_value on overflow. */ >> + STRIP_NOPS (expr_ovl); >> + STRIP_NOPS (result_ovl); >> if ((complain & tf_warning) >> && TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl)) >> overflow_warning (input_location, result_ovl); > > I committed patches for binary & unary operations together with > testcases. Regression-run still running. There seems to be > additional expressions needed in constexpr for this. For now we have > a bootstrap-issue due cast_expr in cxx_eval_constant_expression. > There might be more of them ... I had to add for now that cxx_eval_constant_expr ignores on CAST_EXPR, STATIC_CAST_EXPR, OVERLOAD, and TREE_LIST. Additionally we need to handle for increments that offset is NULL_TREE (TREE_OPERAND (,1)) .
2015-09-01 10:43 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: > 2015-09-01 10:15 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >> 2015-08-31 22:19 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >>> 2015-08-31 21:43 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >>>> 2015-08-31 21:29 GMT+02:00 Jason Merrill <jason@redhat.com>: >>>>> On 08/31/2015 03:08 PM, Kai Tietz wrote: >>>>>> >>>>>> I will need to verify that this patch doesn't introduce regressions. >>>>>> The wacky thing here is the encapsulation of overflowed-arguments in >>>>>> maybe_constant_value function by nop-expr. >>>>> >>>>> >>>>> Do we need to worry about that? If one of the operands is overflowed, we >>>>> don't care whether the result is overflowed. >>>> >>>> Well, we would introduce, if we don't see in condition that operand >>>> already overflowed, double overflow-warning, which seems to be >>>> something we avoided until now. So I would say, it matters. >>>> >>>> Kai >>> >>> Similar to the binary-operation we want to do then the same for >>> unary-operations, too. >>> >>> Eg. testcase: >>> >>> #include <limits.h> >>> >>> constexpr int f() { return INT_MIN; } >>> >>> int main() >>> { >>> return -f(); // { dg-warning "overflow" } >>> } >>> With following patch we do diagnostics for it. >>> >>> Kai >>> >>> Index: semantics.c >>> =================================================================== >>> --- semantics.c (Revision 227339) >>> +++ semantics.c (Arbeitskopie) >>> @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co >>> tree result = build_x_unary_op (loc, code, expr, complain); >>> tree result_ovl = result; >>> >>> - expr_ovl = fold_simple (expr_ovl); >>> - result_ovl = fold_simple (result); >>> - >>> + expr_ovl = maybe_constant_value (expr_ovl); >>> + result_ovl = maybe_constant_value (result); >>> + /* Strip nop-expressions added by maybe_constant_value on overflow. */ >>> + STRIP_NOPS (expr_ovl); >>> + STRIP_NOPS (result_ovl); >>> if ((complain & tf_warning) >>> && TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl)) >>> overflow_warning (input_location, result_ovl); >> >> I committed patches for binary & unary operations together with >> testcases. Regression-run still running. There seems to be >> additional expressions needed in constexpr for this. For now we have >> a bootstrap-issue due cast_expr in cxx_eval_constant_expression. >> There might be more of them ... > > I had to add for now that cxx_eval_constant_expr ignores on CAST_EXPR, > STATIC_CAST_EXPR, OVERLOAD, and TREE_LIST. Additionally we need to > handle for increments that offset is NULL_TREE (TREE_OPERAND (,1)) . Issue was easier to resolve by checking in binary/unary overflow-checking-functions that we aren't processing template declarations. If we aren't within template-declaration processing we can call maybe_constant_value. This avoids that we feed maybe_constant_value with mentioned C++-template specific expressions. Bootstrap ran successful, regression-testing still running for it. I committed additional check already to branch. Kai
2015-09-01 13:17 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: > 2015-09-01 10:43 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >> 2015-09-01 10:15 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >>> 2015-08-31 22:19 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >>>> 2015-08-31 21:43 GMT+02:00 Kai Tietz <ktietz70@googlemail.com>: >>>>> 2015-08-31 21:29 GMT+02:00 Jason Merrill <jason@redhat.com>: >>>>>> On 08/31/2015 03:08 PM, Kai Tietz wrote: >>>>>>> >>>>>>> I will need to verify that this patch doesn't introduce regressions. >>>>>>> The wacky thing here is the encapsulation of overflowed-arguments in >>>>>>> maybe_constant_value function by nop-expr. >>>>>> >>>>>> >>>>>> Do we need to worry about that? If one of the operands is overflowed, we >>>>>> don't care whether the result is overflowed. >>>>> >>>>> Well, we would introduce, if we don't see in condition that operand >>>>> already overflowed, double overflow-warning, which seems to be >>>>> something we avoided until now. So I would say, it matters. >>>>> >>>>> Kai >>>> >>>> Similar to the binary-operation we want to do then the same for >>>> unary-operations, too. >>>> >>>> Eg. testcase: >>>> >>>> #include <limits.h> >>>> >>>> constexpr int f() { return INT_MIN; } >>>> >>>> int main() >>>> { >>>> return -f(); // { dg-warning "overflow" } >>>> } >>>> With following patch we do diagnostics for it. >>>> >>>> Kai >>>> >>>> Index: semantics.c >>>> =================================================================== >>>> --- semantics.c (Revision 227339) >>>> +++ semantics.c (Arbeitskopie) >>>> @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co >>>> tree result = build_x_unary_op (loc, code, expr, complain); >>>> tree result_ovl = result; >>>> >>>> - expr_ovl = fold_simple (expr_ovl); >>>> - result_ovl = fold_simple (result); >>>> - >>>> + expr_ovl = maybe_constant_value (expr_ovl); >>>> + result_ovl = maybe_constant_value (result); >>>> + /* Strip nop-expressions added by maybe_constant_value on overflow. */ >>>> + STRIP_NOPS (expr_ovl); >>>> + STRIP_NOPS (result_ovl); >>>> if ((complain & tf_warning) >>>> && TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl)) >>>> overflow_warning (input_location, result_ovl); >>> >>> I committed patches for binary & unary operations together with >>> testcases. Regression-run still running. There seems to be >>> additional expressions needed in constexpr for this. For now we have >>> a bootstrap-issue due cast_expr in cxx_eval_constant_expression. >>> There might be more of them ... >> >> I had to add for now that cxx_eval_constant_expr ignores on CAST_EXPR, >> STATIC_CAST_EXPR, OVERLOAD, and TREE_LIST. Additionally we need to >> handle for increments that offset is NULL_TREE (TREE_OPERAND (,1)) . > > Issue was easier to resolve by checking in binary/unary > overflow-checking-functions that we aren't processing template > declarations. If we aren't within template-declaration processing we > can call maybe_constant_value. This avoids that we feed > maybe_constant_value with mentioned C++-template specific expressions. > > Bootstrap ran successful, regression-testing still running for it. I > committed additional check already to branch. All tests passed as expected. Kai
Index: semantics.c =================================================================== --- semantics.c (Revision 227339) +++ semantics.c (Arbeitskopie) @@ -2553,9 +2553,11 @@ finish_unary_op_expr (location_t loc, enum tree_co tree result = build_x_unary_op (loc, code, expr, complain); tree result_ovl = result; - expr_ovl = fold_simple (expr_ovl); - result_ovl = fold_simple (result); - + expr_ovl = maybe_constant_value (expr_ovl); + result_ovl = maybe_constant_value (result); + /* Strip nop-expressions added by maybe_constant_value on overflow. */ + STRIP_NOPS (expr_ovl); + STRIP_NOPS (result_ovl); if ((complain & tf_warning) && TREE_OVERFLOW_P (result_ovl) && !TREE_OVERFLOW_P (expr_ovl)) overflow_warning (input_location, result_ovl);