On Mon, 16 Jan 2012, Richard Guenther wrote:

> On Fri, 13 Jan 2012, Joseph S. Myers wrote:
> 
> > On Fri, 13 Jan 2012, Richard Guenther wrote:
> > 
> > > This fixes PR37985 where since 
> > > http://gcc.gnu.org/ml/gcc-patches/2006-08/msg01041.html we
> > > mark conversions produced by convert_to_integer with TREE_NO_WARNING.
> > > This may shadow "real" stmts with no effects, thus we should
> > > simply strip them again before checking for TREE_SIDE_EFFECTS.
> > > 
> > > Bootstrap & regtest pending on x86_64-unknown-linux-gnu.
> > > 
> > > Ok if that passes?
> > 
> > OK.
> 
> It FAILs gcc.dg/20040202-1.c (we warn about a folded memcpy).  It
> looks like wrapping things in TREE_NO_WARNING NOP_EXPRs is
> fragile as soon as you consider multiple warning kinds (thus, of
> course the patch causing this regression is at fault).
> 
> OTOH for the regression warning on folded stmts I really wonder
> why c_process_expr_stmt folds at all before emitting warnings - why
> intentionally divert from the source AST here?  I'm testing
> the following before giving up on this regression (which solves
> the gcc.dg/20040202-1.c FAIL at least).

Which works besides now FAILing gcc.dg/strncpy-fix-1.c.  We get
a "value computed is not used" warning because we are still folding
the strncpy to a memcpy because we fold all builtin calls at the very
point they are created (build_function_call_vec):

  if (name != NULL_TREE
      && !strncmp (IDENTIFIER_POINTER (name), "__builtin_", 10))
    {
      if (require_constant_value)
        result =
          fold_build_call_array_initializer_loc (loc, TREE_TYPE (fntype),
                                                 function, nargs, 
argarray);
      else
        result = fold_build_call_array_loc (loc, TREE_TYPE (fntype),
                                            function, nargs, argarray);

"Fixed" by not doing that if !require_constant_value.  We'll fold
it during the c_fully_fold call anyway (and during gimplification again).

Re-testing as follows.

Richard.

2012-01-13  Richard Guenther  <rguent...@suse.de>

        PR c/37985
        * c-typeck.c (emit_side_effect_warnings): Strip conversions
        with TREE_NO_WARNING.
        (c_process_expr_stmt): Fold the stmt after emitting warnings.
        (build_function_call_vec): Do not fold calls here if we do not
        require a constant value.

        * gcc.dg/Wunused-value-4.c: New testcase.

Index: gcc/testsuite/gcc.dg/Wunused-value-4.c
===================================================================
*** gcc/testsuite/gcc.dg/Wunused-value-4.c      (revision 0)
--- gcc/testsuite/gcc.dg/Wunused-value-4.c      (revision 0)
***************
*** 0 ****
--- 1,9 ----
+ /* PR c/37985 */
+ /* { dg-do compile } */
+ /* { dg-options "-Wunused-value" } */
+ 
+ unsigned char foo(unsigned char a)
+ {
+   a >> 2; /* { dg-warning "statement with no effect" } */
+   return a;
+ }
Index: gcc/c-typeck.c
===================================================================
*** gcc/c-typeck.c      (revision 183205)
--- gcc/c-typeck.c      (working copy)
*************** build_function_call_vec (location_t loc,
*** 2826,2841 ****
    /* Check that the arguments to the function are valid.  */
    check_function_arguments (fntype, nargs, argarray);
  
!   if (name != NULL_TREE
        && !strncmp (IDENTIFIER_POINTER (name), "__builtin_", 10))
      {
!       if (require_constant_value)
!       result =
!         fold_build_call_array_initializer_loc (loc, TREE_TYPE (fntype),
!                                                function, nargs, argarray);
!       else
!       result = fold_build_call_array_loc (loc, TREE_TYPE (fntype),
!                                           function, nargs, argarray);
        if (TREE_CODE (result) == NOP_EXPR
          && TREE_CODE (TREE_OPERAND (result, 0)) == INTEGER_CST)
        STRIP_TYPE_NOPS (result);
--- 2826,2838 ----
    /* Check that the arguments to the function are valid.  */
    check_function_arguments (fntype, nargs, argarray);
  
!   if (require_constant_value
!       && name != NULL_TREE
        && !strncmp (IDENTIFIER_POINTER (name), "__builtin_", 10))
      {
!       result =
!       fold_build_call_array_initializer_loc (loc, TREE_TYPE (fntype),
!                                              function, nargs, argarray);
        if (TREE_CODE (result) == NOP_EXPR
          && TREE_CODE (TREE_OPERAND (result, 0)) == INTEGER_CST)
        STRIP_TYPE_NOPS (result);
*************** c_finish_bc_stmt (location_t loc, tree *
*** 9163,9168 ****
--- 9160,9169 ----
  static void
  emit_side_effect_warnings (location_t loc, tree expr)
  {
+   /* Strip conversions marked with TREE_NO_WARNING.  */
+   while (TREE_NO_WARNING (expr) && CONVERT_EXPR_P (expr))
+     expr = TREE_OPERAND (expr, 0);
+ 
    if (expr == error_mark_node)
      ;
    else if (!TREE_SIDE_EFFECTS (expr))
*************** c_process_expr_stmt (location_t loc, tre
*** 9186,9193 ****
    if (!expr)
      return NULL_TREE;
  
-   expr = c_fully_fold (expr, false, NULL);
- 
    if (warn_sequence_point)
      verify_sequence_points (expr);
  
--- 9187,9192 ----
*************** c_process_expr_stmt (location_t loc, tre
*** 9213,9218 ****
--- 9212,9219 ----
        || TREE_CODE (exprv) == ADDR_EXPR)
      mark_exp_read (exprv);
  
+   expr = c_fully_fold (expr, false, NULL);
+ 
    /* If the expression is not of a type to which we cannot assign a line
       number, wrap the thing in a no-op NOP_EXPR.  */
    if (DECL_P (expr) || CONSTANT_CLASS_P (expr))

Reply via email to