This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] PR c/456: C90 warning for non-constant initializers


The following patch is my attempt to fix the long standing PR c/456,
which is a failure to issue a required diagnostic initializing static
variables with "non-constant" expressions.  I started out trying to
discover why gcc.dg/c90-const-expr-3.c is current failing on mainline,
but I got side tracked into resolving the XFAILs in c90-const-expr-1.c!

The problem is that we fail to issue a warning for constructs such as:

  static int x = (1 ? 0 : foo ());

where in C90 the initializer isn't/shouldn't be considered constant.
The problem is simply that "fold" is doing it's thing, and reducing
the above expression at compile-time to zero, which prevents the C
front-end from issuing a warning.  Compile-time folding of COND_EXPR
in initializer is apparently allowed in C99, but alas not in C90.

At first I thought that this would be a fairly simple problem to fix,
using exactly the same idiom as recently used to resolve PR c/14649,
and adding an additional test to c-typeck's build_conditional_expr
that recognized when we'd folded a COND_EXPR with a non-constant
operand into a "constant" tree.

Unfortunately, however, although this fixes gcc.dg/c90-const-expr-1.c,
this change alone regresses gcc.dg/bconstp-1.c, which tests a GNU C
extension, namely:

  static int x = (__builtin_constant_p (0) ? 0 : foo ());

i.e. as an extension we allow to constant folding of COND_EXPRs without
warning, when the conditional expression is GCC's __builtin_constant_p!
The problem then is that by the time "fold" has optimized the ternary
operators operands, we need to distinguish "1 ? X : Y" from "1 ? X : Y".


The inovative if controversial solution, that I propose below is the
use (locally to the C front-end) of signaling and non-signaling integer
constants.  Its already the case that the middle-end's fold_builtin
function tags trees that its managed to fold with TREE_NO_WARNING.
Using this flag, we can distinguish the "1"s generated by constant
folding GCC builtins from the "1"s in the user's code.  My solution
to PR c/14649 would already issue a diagnostic if we the builtin wasn't
called in the __builtin_foo form, so this "bit" fits out purposes
perfectly.

Unfortunately, for constant integer values returned from fold_builtin,
to avoid clobbering the shared integer_one_node, we're wrapping the
INTEGER_CST in a NOP_EXPR and setting the TREE_NO_WARNING flag on it.
Alas, the use of a NOP_EXPR means that this doesn't survive the C
parser, where "default_conversions" strips NOP_EXPRs and then recreates
them based on the final type.

The tweak below is that rather than create a fragile NOP_EXPR that
would be eliminated by the C front-end (or "fold"), we perform a
(shallow) copy of the INTEGER_CST node, and directly set the
TREE_NO_WARNING bit on this.  One extra tweak, is the required to
the C front-end's c_common_truthvalue_conversion, such that we
perserve the "signaling one" and "signaling zero".

The good news: It works!


The following patch has been tested on i686-pc-linux-gnu with a full
"make bootstrap", all default languages, and regression tested with a
top-level "make -k check" with no new failures.  The resolution of
gcc.dg/c90-const-expr-1.c turns five XFAILs into PASSes.

Ok for mainline?


2004-06-23  Roger Sayle  <roger@eyesopen.com>

	PR c/456
	* builtins.c (fold_builtin): Place the TREE_NO_WARNING marker on
	a copy of a constant node, instead of on a NOP_EXPR wrapping it.
	* c-common.c (c_common_truthvalue_conversion): Preserve the
	TREE_NO_WARNING flag on truth conversions of integer constants.
	* c-typeck.c (build_conditional_expr): Issue a pedwarn in ISO C90,
	if we constant fold away a side-effecting branch of a conditional
	expression in a constant initializer.

	* gcc.dg/c90-const-expr-1.c: Remove XFAILs.


? fastjar/fastjar.info
Index: gcc/builtins.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/builtins.c,v
retrieving revision 1.342
diff -c -3 -p -r1.342 builtins.c
*** gcc/builtins.c	20 Jun 2004 17:03:02 -0000	1.342
--- gcc/builtins.c	23 Jun 2004 15:15:43 -0000
*************** fold_builtin (tree exp)
*** 8276,8282 ****
      {
        /* ??? Don't clobber shared nodes such as integer_zero_node.  */
        if (TREE_CODE_CLASS (TREE_CODE (exp)) == 'c')
! 	exp = build1 (NOP_EXPR, TREE_TYPE (exp), exp);
        TREE_NO_WARNING (exp) = 1;
      }
    return exp;
--- 8276,8282 ----
      {
        /* ??? Don't clobber shared nodes such as integer_zero_node.  */
        if (TREE_CODE_CLASS (TREE_CODE (exp)) == 'c')
! 	exp = copy_node (exp);
        TREE_NO_WARNING (exp) = 1;
      }
    return exp;
Index: gcc/c-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.c,v
retrieving revision 1.518
diff -c -3 -p -r1.518 c-common.c
*** gcc/c-common.c	21 Jun 2004 09:15:20 -0000	1.518
--- gcc/c-common.c	23 Jun 2004 15:15:48 -0000
*************** c_common_truthvalue_conversion (tree exp
*** 2449,2455 ****
        return expr;

      case INTEGER_CST:
!       return integer_zerop (expr) ? truthvalue_false_node : truthvalue_true_node;

      case REAL_CST:
        return real_zerop (expr) ? truthvalue_false_node : truthvalue_true_node;
--- 2449,2465 ----
        return expr;

      case INTEGER_CST:
!       /* The following preserves TREE_NO_WARNING on INTEGER_CST.  */
!       if (!TREE_NO_WARNING (expr))
! 	return integer_zerop (expr) ? truthvalue_false_node
! 				    : truthvalue_true_node;
!       if (TREE_TYPE (expr) == truthvalue_type_node)
! 	return expr;
!       expr = integer_zerop (expr) ? truthvalue_false_node
! 				  : truthvalue_true_node;
!       expr = copy_node (expr);
!       TREE_NO_WARNING (expr) = 1;
!       return expr;

      case REAL_CST:
        return real_zerop (expr) ? truthvalue_false_node : truthvalue_true_node;
Index: gcc/c-typeck.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-typeck.c,v
retrieving revision 1.323
diff -c -3 -p -r1.323 c-typeck.c
*** gcc/c-typeck.c	21 Jun 2004 09:15:20 -0000	1.323
--- gcc/c-typeck.c	23 Jun 2004 15:15:54 -0000
*************** build_conditional_expr (tree ifexp, tree
*** 2736,2741 ****
--- 2736,2742 ----
    enum tree_code code2;
    tree result_type = NULL;
    tree orig_op1 = op1, orig_op2 = op2;
+   tree result;

    ifexp = lang_hooks.truthvalue_conversion (default_conversion (ifexp));

*************** build_conditional_expr (tree ifexp, tree
*** 2888,2896 ****
      op2 = convert_and_check (result_type, op2);

    if (TREE_CODE (ifexp) == INTEGER_CST)
!     return non_lvalue (integer_zerop (ifexp) ? op2 : op1);

!   return fold (build (COND_EXPR, result_type, ifexp, op1, op2));
  }

  /* Given a list of expressions, return a compound expression
--- 2889,2908 ----
      op2 = convert_and_check (result_type, op2);

    if (TREE_CODE (ifexp) == INTEGER_CST)
!     result = non_lvalue (integer_zerop (ifexp) ? op2 : op1);
!   else
!     result = fold (build3 (COND_EXPR, result_type, ifexp, op1, op2));
!
!   /* In C90, warn if we've optimized away a non-constant expression in
!      an initializer.  */
!   if (require_constant_value
!       && !flag_isoc99
!       && (!TREE_CONSTANT (op1) || !TREE_CONSTANT (op2))
!       && TREE_CONSTANT (result)
!       && !TREE_NO_WARNING (ifexp))
!     pedwarn_init ("initializer element is not constant");

!   return result;
  }

  /* Given a list of expressions, return a compound expression


Index: gcc/testsuite/gcc.dg/c90-const-expr-1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/gcc.dg/c90-const-expr-1.c,v
retrieving revision 1.1
diff -c -3 -p -r1.1 c90-const-expr-1.c
*** gcc/testsuite/gcc.dg/c90-const-expr-1.c	9 Aug 2000 01:05:09 -0000	1.1
--- gcc/testsuite/gcc.dg/c90-const-expr-1.c	23 Jun 2004 15:57:51 -0000
*************** void
*** 15,23 ****
  foo (void)
  {
    int i;
!   static int j = (1 ? 0 : (i = 2)); /* { dg-error "initial" "assignment" { xfail *-*-* } } */
!   static int k = (1 ? 0 : ++i); /* { dg-error "initial" "increment" { xfail *-*-* } } */
!   static int l = (1 ? 0 : --i); /* { dg-error "initial" "decrement" { xfail *-*-* } } */
!   static int m = (1 ? 0 : bar ()); /* { dg-error "initial" "function call" { xfail *-*-* } } */
!   static int n = (1 ? 0 : (2, 3)); /* { dg-error "initial" "comma" { xfail *-*-* } } */
  }
--- 15,23 ----
  foo (void)
  {
    int i;
!   static int j = (1 ? 0 : (i = 2)); /* { dg-error "initial" "assignment" } */
!   static int k = (1 ? 0 : ++i); /* { dg-error "initial" "increment" } */
!   static int l = (1 ? 0 : --i); /* { dg-error "initial" "decrement" } */
!   static int m = (1 ? 0 : bar ()); /* { dg-error "initial" "function call" } */
!   static int n = (1 ? 0 : (2, 3)); /* { dg-error "initial" "comma" } */
  }


Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]