improve constexpr handling

Here comes the greatly enhanced v3 of this series.

Luc's suggestions about splitting some patches turned out to be very fruitful!

Former [01/13] ("expression: introduce additional expression constness tracking flags") has been split into
- [01/21] ("expression: introduce additional expression constness tracking flags")
- [02/21] ("expression: init constexpr_flags at expression allocation")
- [07/21] ("expression: add support for tagging arithmetic constant expressions")

In particular, the introduction of the arithmetic constant expression flag is deferred until the last one of these three. This addresses Luc's concerns that the arithmetic constant expression handlings within the v2 "examine constness of XXX at evaluation only"-patches should better get split off into a separate patch each.

Former [06/13] ("expression, evaluate: add support for recognizing address constants") has been split into
- [08/21] ("expression, evaluate: add support for tagging address constants")
- [10/21] ("expression, evaluate: recognize static objects as address constants")
- [12/21] ("evaluate: recognize address constants created through pointer arithmetic")
- [13/21] ("evaluate: recognize members of static compound objects as address constants")
- [14/21] ("evaluate: recognize string literals as address constants")

The address constant handling part from former
[02/13] ("expression: examine constness of casts at evaluation only") has been extracted into a patch on its own, namely
[11/21] ("evaluate: recognize address constants created through casts")

Patches [10-14/21] have been placed just until after static initializer checking has been introduced in
[09/21] ("evaluate: check static storage duration objects' intializers' constness"). This way, the monster testcase from former
[07/13] ("evaluate: check static storage duration objects' intializers' constness") can be split and the individual tests attached to the different patches as appropriate.

Note that we still have got this
[20/21] ("symbol: do not inherit storage modifiers from base types at examination") thing.

Detailed changes:
[01/21] ("expression: introduce additional expression constness tracking flags"):
- The expr_{set,clear}_flag() helpers have been replaced by bitmasks to be ored in or anded outnow.
- Renamed ->flags to ->constexpr_flags
- ->constexpr_flags initialization at expression allocation has been extracted into
[02/21] ("expression: init constexpr_flags at expression allocation")
- neither of the arithmetic constant expression flag nor address nor the address constant flag gets introduced by this patch anymore.
They get introduced in
[07/21] ("expression: add support for tagging arithmetic constant expressions") [08/21] ("expression, evaluate: add support for tagging address constants") as needed.
- Luc's remarks about the 'sic' comments carried over from the original code have been addressed in the new
[21/21] ("evaluation: treat comparsions between types as integer constexpr")
- Changes to patch description as suggested by Luc.
- Tag a TOKEN_ZERO_IDENT by integer constant instead of integer constant expression. Feels more natural but should not matter.

[02/21] ("expression: init constexpr_flags at expression allocation")
This one is new. Split off from former [01/13].

[03-06/21] ("expression: examine constness of XXX at evaluation only")
Adjusted patch descriptions as suggested by Luc.

[07/21] ("expression: add support for tagging arithmetic constant expressions")
This one is new. Split off from former [01/13].

[08/21] ("expression, evaluate: add support for tagging address constants")
Split off actual recognition of various address constant forms into separate patches [10-14/21]

[09/21] ("evaluate: check static storage duration objects' intializers' constness").
- -W-flag has been renamed to -Wconstexpr-not-const
- the associated warning message has been shortened to
"non-constant initializer for static object".
- the attached testcase has been greatly reduced as the tests are now

    attached to the patches [10-14/21].

[10-14/21]
These ones are new and split off from former
[06/13] ("expression, evaluate: add support for tagging address constants") They implement actual address constant recognition functionality.

In
[12/21] ("evaluate: recognize address constants created through pointer arithmetic") a whitespace issue from former
[06/13] ("expression, evaluate: add support for recognizing address constants") in an if-clause has been fixed.

[15/21] ("expression: recognize references to labels as address constants")
A testcase for has been added.

[19/21] ("evaluate: relax some constant expression rules for pointer expressions")
A typo ("adress") has been fixed.

[21/21] ("evaluation: treat comparsions between types as integer constexpr")
This one is new.

Nicolai Stange (21):
expression: introduce additional expression constness tracking flags
expression: init constexpr_flags at expression allocation
expression: examine constness of casts at evaluation only
expression: examine constness of binops and alike at evaluation only
expression: examine constness of preops at evaluation only
expression: examine constness of conditionals at evaluation only
expression: add support for tagging arithmetic constant expressions expression, evaluate: add support for tagging address constants
evaluate: check static storage duration objects' intializers'
constness
expression, evaluate: recognize static objects as address constants
evaluate: recognize address constants created through casts
evaluate: recognize address constants created through pointer
arithmetic
evaluate: recognize members of static compound objects as address
constants
evaluate: recognize string literals as address constants
expression: recognize references to labels as address constants
expression: examine constness of __builtin_offsetof at evaluation only
symbol: flag builtins constant_p, safe_p and warning as constexprs
evaluate: relax some constant expression rules for pointer expressions expression, evaluate: support compound literals as address constants
symbol: do not inherit storage modifiers from base types at
examination
evaluation: treat comparsions between types as integer constexpr

 evaluate.c                                   | 207 +++++++++++++++++++++------
 expand.c                                     |  10 +-
 expression.c                                 |  50 +++----
 expression.h                                 |  83 ++++++++++-
 lib.c                                        |   2 +
 lib.h                                        |   1 +
 sparse.1                                     |   9 ++
 symbol.c                                     |  12 +-
 validation/constexpr-addr-of-static-member.c |  26 ++++
 validation/constexpr-addr-of-static.c        |  36 +++++
 validation/constexpr-binop.c                 |  33 +++++
 validation/constexpr-cast.c                  |  25 ++++
 validation/constexpr-compound-literal.c      |  19 +++
 validation/constexpr-conditional.c           |  34 +++++
 validation/constexpr-init.c                  |  60 ++++++++
 validation/constexpr-labelref.c              |  15 ++
 validation/constexpr-offsetof.c              |  21 +++
 validation/constexpr-pointer-arith.c         |  28 ++++
 validation/constexpr-pointer-cast.c          |  13 ++
 validation/constexpr-preop.c                 |  29 ++++
 validation/constexpr-string.c                |   9 ++
 validation/constexpr-types-compatible-p.c    |   9 ++
 22 files changed, 640 insertions(+), 91 deletions(-)
 create mode 100644 validation/constexpr-addr-of-static-member.c
 create mode 100644 validation/constexpr-addr-of-static.c
 create mode 100644 validation/constexpr-binop.c
 create mode 100644 validation/constexpr-cast.c
 create mode 100644 validation/constexpr-compound-literal.c
 create mode 100644 validation/constexpr-conditional.c
 create mode 100644 validation/constexpr-init.c
 create mode 100644 validation/constexpr-labelref.c
 create mode 100644 validation/constexpr-offsetof.c
 create mode 100644 validation/constexpr-pointer-arith.c
 create mode 100644 validation/constexpr-pointer-cast.c
 create mode 100644 validation/constexpr-preop.c
 create mode 100644 validation/constexpr-string.c
 create mode 100644 validation/constexpr-types-compatible-p.c

2.7.0

Even if sparse attempted to verify that initializers for static storage duration objects are constant expressions [6.7.8(4)] (which it currently does not), it could not tell reliably.
Example:

  enum { b = 0 };
  static void *c = { (void*)b }; /* disallowed by C99 */

References to enum members are not allowed in address constants [6.6(9)] and thus, the initializer is not a constant expression at all.

Prepare for a more fine-grained tracking of expression constness in the sense of C99 [6.4.4, 6.6].

Introduce a broader set of constness tracking flags, resembling the four types of primary expression constants [6.4.4] (integer, floating, enumeration, character). Define helper macros to consistently set and clear these flags as they are not completely independent.

In particular, introduce the following flags for tagging expression constness at the level of primary expressions:
- CONSTEXPR_FLAG_INT_CONST: integer constant, i.e. literal
- CONSTEXPR_FLAG_FP_CONST: floating point constant, equivalent to the former Float_literal flag
- CONSTEXPR_FLAG_ENUM_CONST: enumeration constant
- CONSTEXPR_FLAG_CHAR_CONST: character constant

Introduce the CONSTEXPR_FLAG_INT_CONST_EXPR flag meant for tagging integer constant expressions. It is equivalent to the former Int_const_expr flag.
Note that the new CONSTEXPR_FLAG_INT_CONST, CONSTEXPR_FLAG_ENUM_CONST and CONSTEXPR_FLAG_CHAR_CONST flags imply CONSTEXPR_FLAG_INT_CONST_EXPR being set.

Finally, rename ->flags to ->constexpr_flags because they are solely used for the purpose of tracking an expression's constness.

---
 evaluate.c   | 83 ++++++++++++++++++++++++++++++++++++------------------------
 expand.c     | 10 ++++----
 expression.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------------
 expression.h | 65 +++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 171 insertions(+), 69 deletions(-)

index 035e448..7bce4fb 100644
@@ -323,7 +323,7 @@ static struct expression * cast_to(struct expression *old, struct symbol *type)
 	}
 
 	expr = alloc_expression(old->pos, EXPR_IMPLIED_CAST);
-	expr->flags = old->flags;
+	expr->constexpr_flags = old->constexpr_flags;
 	expr->ctype = type;
 	expr->cast_type = type;
 	expr->cast_expression = old;

@@ -400,7 +400,7 @@ static struct symbol *bad_expr_type(struct expression *expr)
 		break;
 	}
 
-	expr->flags = 0;
+	expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	return expr->ctype = &bad_ctype;
 }
 

@@ -879,9 +879,11 @@ static struct symbol *evaluate_logical(struct expression *expr)
 
 	/* the result is int [6.5.13(3), 6.5.14(3)] */
 	expr->ctype = &int_ctype;
-	if (expr->flags) {
-		if (!(expr->left->flags & expr->right->flags & Int_const_expr))
-			expr->flags = 0;
+	if (expr->constexpr_flags) {
+		if (!(expr->left->constexpr_flags &
+				expr->right->constexpr_flags &
+				CONSTEXPR_FLAG_INT_CONST_EXPR))
+			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	}
 	return &int_ctype;
 }

@@ -893,9 +895,11 @@ static struct symbol *evaluate_binop(struct expression *expr)
 	int rclass = classify_type(expr->right->ctype, &rtype);
 	int op = expr->op;
 
-	if (expr->flags) {
-		if (!(expr->left->flags & expr->right->flags & Int_const_expr))
-			expr->flags = 0;
+	if (expr->constexpr_flags) {
+		if (!(expr->left->constexpr_flags &
+				expr->right->constexpr_flags &
+				CONSTEXPR_FLAG_INT_CONST_EXPR))
+			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	}
 
 	/* number op number */

@@ -965,7 +969,8 @@ static struct symbol *evaluate_comma(struct expression *expr)
 	expr->ctype = degenerate(expr->right);
 	if (expr->ctype == &null_ctype)
 		expr->ctype = &ptr_ctype;
-	expr->flags &= expr->left->flags & expr->right->flags;
+	expr->constexpr_flags &= expr->left->constexpr_flags &
+		expr->right->constexpr_flags;
 	return expr->ctype;
 }
 

@@ -986,7 +991,7 @@ static inline int is_null_pointer_constant(struct expression *e)
 {
 	if (e->ctype == &null_ctype)
 		return 1;
-	if (!(e->flags & Int_const_expr))
+	if (!(e->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR))
 		return 0;
 	return is_zero_constant(e) ? 2 : 0;
 }

@@ -1000,9 +1005,11 @@ static struct symbol *evaluate_compare(struct expression *expr)
 	struct symbol *ctype;
 	const char *typediff;
 
-	if (expr->flags) {
-		if (!(expr->left->flags & expr->right->flags & Int_const_expr))
-			expr->flags = 0;
+	if (expr->constexpr_flags) {
+		if (!(expr->left->constexpr_flags &
+				expr->right->constexpr_flags &
+				CONSTEXPR_FLAG_INT_CONST_EXPR))
+			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	}
 
 	/* Type types? */

@@ -1119,11 +1126,13 @@ static struct symbol *evaluate_conditional_expression(struct expression *expr)
 		true = &expr->cond_true;
 	}
 
-	if (expr->flags) {
-		int flags = expr->conditional->flags & Int_const_expr;
-		flags &= (*true)->flags & expr->cond_false->flags;
+	if (expr->constexpr_flags) {
+		int flags = (expr->conditional->constexpr_flags &
+			CONSTEXPR_FLAG_INT_CONST_EXPR);
+		flags &= (*true)->constexpr_flags &
+			expr->cond_false->constexpr_flags;
 		if (!flags)
-			expr->flags = 0;
+			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	}
 
 	lclass = classify_type(ltype, &ltype);

@@ -1681,7 +1690,7 @@ static struct symbol *evaluate_addressof(struct expression *expr)
 	}
 	ctype = op->ctype;
 	*expr = *op->unop;
-	expr->flags = 0;
+	expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 
 	if (expr->type == EXPR_SYMBOL) {
 		struct symbol *sym = expr->symbol;

@@ -1709,7 +1718,7 @@ static struct symbol *evaluate_dereference(struct expression *expr)
 	/* Simplify: *&(expr) => (expr) */
 	if (op->type == EXPR_PREOP && op->op == '&') {
 		*expr = *op->unop;
-		expr->flags = 0;
+		expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 		return expr->ctype;
 	}
 

@@ -1799,8 +1808,9 @@ static struct symbol *evaluate_sign(struct expression *expr)
 {
 	struct symbol *ctype = expr->unop->ctype;
 	int class = classify_type(ctype, &ctype);
-	if (expr->flags && !(expr->unop->flags & Int_const_expr))
-		expr->flags = 0;
+	if (expr->constexpr_flags && !(expr->unop->constexpr_flags &
+					CONSTEXPR_FLAG_INT_CONST_EXPR))
+		expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	/* should be an arithmetic type */
 	if (!(class & TYPE_NUM))
 		return bad_expr_type(expr);

@@ -1854,8 +1864,9 @@ static struct symbol *evaluate_preop(struct expression *expr)
 		return evaluate_postop(expr);
 
 	case '!':
-		if (expr->flags && !(expr->unop->flags & Int_const_expr))
-			expr->flags = 0;
+		if (expr->constexpr_flags && !(expr->unop->constexpr_flags &
+					CONSTEXPR_FLAG_INT_CONST_EXPR))
+			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 		if (is_safe_type(ctype))
 			warning(expr->pos, "testing a 'safe expression'");
 		if (is_float_type(ctype)) {

@@ -2736,12 +2747,13 @@ static struct symbol *evaluate_cast(struct expression *expr)
 
 	/* cast to non-integer type -> not an integer constant expression */
 	if (!is_int(class1))
-		expr->flags = 0;
+		expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	/* if argument turns out to be not an integer constant expression *and*
 	   it was not a floating literal to start with -> too bad */
-	else if (expr->flags == Int_const_expr &&
-		!(target->flags & Int_const_expr))
-		expr->flags = 0;
+	else if (expr->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR &&
+		!(target->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR))
+		expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
+
 	/*
 	 * You can always throw a value away by casting to
 	 * "void" - that's an implicit "force". Note that

@@ -2803,7 +2815,7 @@ static struct symbol *evaluate_cast(struct expression *expr)
 			"cast adds address space to expression (<asn:%d>)", as1);
 
 	if (!(t1->ctype.modifiers & MOD_PTRINHERIT) && class1 == TYPE_PTR &&
-	    !as1 && (target->flags & Int_const_expr)) {
+	    !as1 && (target->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR)) {
 		if (t1->ctype.base_type == &void_ctype) {
 			if (is_zero_constant(target)) {
 				/* NULL */

@@ -2933,7 +2945,7 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
 		}
 		ctype = field;
 		expr->type = EXPR_VALUE;
-		expr->flags = Int_const_expr;
+		expr->constexpr_flags = CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 		expr->value = offset;
 		expr->taint = 0;
 		expr->ctype = size_t_ctype;

@@ -2951,7 +2963,8 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
 		ctype = ctype->ctype.base_type;
 		if (!expr->index) {
 			expr->type = EXPR_VALUE;
-			expr->flags = Int_const_expr;
+			expr->constexpr_flags =
+				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			expr->value = 0;
 			expr->taint = 0;
 			expr->ctype = size_t_ctype;

@@ -2968,13 +2981,15 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
 			m = alloc_const_expression(expr->pos,
 						   bits_to_bytes(ctype->bit_size));
 			m->ctype = size_t_ctype;
-			m->flags = Int_const_expr;
+			m->constexpr_flags |=
+				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			expr->type = EXPR_BINOP;
 			expr->left = idx;
 			expr->right = m;
 			expr->op = '*';
 			expr->ctype = size_t_ctype;
-			expr->flags = m->flags & idx->flags & Int_const_expr;
+			expr->constexpr_flags = m->constexpr_flags &
+				idx->constexpr_flags;
 		}
 	}
 	if (e) {

@@ -2985,7 +3000,9 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
 		if (!evaluate_expression(e))
 			return NULL;
 		expr->type = EXPR_BINOP;
-		expr->flags = e->flags & copy->flags & Int_const_expr;
+		expr->constexpr_flags = e->constexpr_flags &
+			copy->constexpr_flags &
+			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 		expr->op = '+';
 		expr->ctype = size_t_ctype;
 		expr->left = copy;

index 0f6720c..4739d54 100644
@@ -472,11 +472,11 @@ static int expand_comma(struct expression *expr)
 	cost = expand_expression(expr->left);
 	cost += expand_expression(expr->right);
 	if (expr->left->type == EXPR_VALUE || expr->left->type == EXPR_FVALUE) {
-		unsigned flags = expr->flags;
+		unsigned flags = expr->constexpr_flags;
 		unsigned taint;
 		taint = expr->left->type == EXPR_VALUE ? expr->left->taint : 0;
 		*expr = *expr->right;
-		expr->flags = flags;
+		expr->constexpr_flags = flags;
 		if (expr->type == EXPR_VALUE)
 			expr->taint |= Taint_comma | taint;
 	}

@@ -540,14 +540,14 @@ static int expand_conditional(struct expression *expr)
 
 	cond_cost = expand_expression(cond);
 	if (cond->type == EXPR_VALUE) {
-		unsigned flags = expr->flags;
+		unsigned flags = expr->constexpr_flags;
 		if (!cond->value)
 			true = false;
 		if (!true)
 			true = cond;
 		cost = expand_expression(true);
 		*expr = *true;
-		expr->flags = flags;
+		expr->constexpr_flags = flags;
 		if (expr->type == EXPR_VALUE)
 			expr->taint |= cond->taint;
 		return cost;

@@ -1212,7 +1212,7 @@ static int expand_statement(struct statement *stmt)
 
 static inline int bad_integer_constant_expression(struct expression *expr)
 {
-	if (!(expr->flags & Int_const_expr))
+	if (!(expr->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR))
 		return 1;
 	if (expr->taint & Taint_comma)
 		return 1;

2.7.0

The changes are, in themselves, fine to me but I have a few remarks:

*) I think the patch would be nicer (abd certainly easier to review) if
it would be splitted so that changes that can't possibly go wrong are not mixed with others which changes behaviour/semantics. So you can mechanically replace Int_const_expr by CONSTEXPR_... then ->flags by ...
And only then replace ..._EXPR by ..._SET_MASK and so on

*) you will probably hate me for this but ...
I think that the names you're using are way too long.
It doesn't help readability at all, especially when because of the length you need to fole lines in if-expression 'CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK', that's already really long and the change 'flags' to 'constexpr_flags' doesn't help either

Currently, the expression evaluation code explicitly opts out from constness at evaluation if certain criteria regarding the subexpressions are not matched.

Instead of this active opt-out, we want to have subexpression constness attributes to get propagated from child expressions to their parents in the future.

A prerequisite is that each expression's ->constexpr_flags is in a defined state at all times.

Set ->constexpr_flags to CONSTEXPR_FLAG_NONE at expression allocation time, i.e. in alloc_expression() as well as in alloc_const_expression().

---
 expression.h | 2 ++
 1 file changed, 2 insertions(+)

index 3725a73..8a943a4 100644
@@ -255,6 +255,7 @@ static inline struct expression *alloc_expression(struct position pos, int type)
 	struct expression *expr = __alloc_expression(0);
 	expr->type = type;
 	expr->pos = pos;
+	expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	return expr;
 }
 

@@ -265,6 +266,7 @@ static inline struct expression *alloc_const_expression(struct position pos, int
 	expr->pos = pos;
 	expr->value = value;
 	expr->ctype = &int_ctype;
+	expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 	return expr;
 }
 

2.7.0

Move the whole calculation of cast expressions' constness flags to the evaluation phase such that expressions like

(int)__builtin_choose_expr(0, 0, 0)

can now be recognized as qualifying as integer constant expressions.

---
 evaluate.c                  | 27 ++++++++++++++++++---------
 expression.c                | 21 ---------------------
 expression.h                | 10 ++++++++++
 validation/constexpr-cast.c | 25 +++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 30 deletions(-)
 create mode 100644 validation/constexpr-cast.c

index 7bce4fb..24a7f2f 100644
@@ -323,7 +323,6 @@ static struct expression * cast_to(struct expression *old, struct symbol *type)
 	}
 
 	expr = alloc_expression(old->pos, EXPR_IMPLIED_CAST);
-	expr->constexpr_flags = old->constexpr_flags;
 	expr->ctype = type;
 	expr->cast_type = type;
 	expr->cast_expression = old;

@@ -2745,14 +2744,24 @@ static struct symbol *evaluate_cast(struct expression *expr)
 
 	class1 = classify_type(ctype, &t1);
 
-	/* cast to non-integer type -> not an integer constant expression */
-	if (!is_int(class1))
-		expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
-	/* if argument turns out to be not an integer constant expression *and*
-	   it was not a floating literal to start with -> too bad */
-	else if (expr->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR &&
-		!(target->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR))
-		expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
+	if (class1 & TYPE_NUM) {
+		expr->constexpr_flags = target->constexpr_flags &
+			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
+		/*
+		 * Cast to float type -> not an integer constant
+		 * expression [6.6(6)].
+		 */
+		if (class1 & TYPE_FLOAT)
+			expr->constexpr_flags &=
+				~CONSTEXPR_FLAG_INT_CONST_EXPR_CLEAR_MASK;
+		/*
+		 * Casts of float literals to integer type results in
+		 * a constant integer expression [6.6(6)].
+		 */
+		else if (target->constexpr_flags & CONSTEXPR_FLAG_FP_CONST)
+			expr->constexpr_flags =
+				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
+	}
 
 	/*
 	 * You can always throw a value away by casting to

index f9adab6..759bee8 100644
@@ -730,27 +730,6 @@ static struct token *cast_expression(struct token *token, struct expression **tr
 			if (!v)
 				return token;
 			cast->cast_expression = v;
-
-			cast->constexpr_flags = v->constexpr_flags &
-				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
-			/*
-			 * Up to now, we missed the (int).0 case here
-			 * which should really get a
-			 * CONSTEXPR_FLAG_INT_CONST_EXPR marker. Also,
-			 * conversion to non-numeric types is not
-			 * properly reflected up to this point.
-			 * However, we do not know until evaluation.
-			 * For the moment, in order to preserve
-			 * semantics, speculatively set
-			 * CONSTEXPR_FLAG_INT_CONST_EXPR if
-			 * CONSTEXPR_FLAG_FP_CONST is
-			 * set. evaluate_cast() will unset
-			 * inappropriate flags again after examining
-			 * type information.
-			 */
-			if (v->constexpr_flags & CONSTEXPR_FLAG_FP_CONST)
-				cast->constexpr_flags |=
-					CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			return token;
 		}
 	}

index 8a943a4..408382e 100644
@@ -119,6 +119,16 @@ enum constexpr_flag {
 	(CONSTEXPR_FLAG_CHAR_CONST | CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK)
 
 /*
+ * not an integer constant expression => neither of integer,
+ * enumeration and character constant
+*/
+#define CONSTEXPR_FLAG_INT_CONST_EXPR_CLEAR_MASK \
+	(CONSTEXPR_FLAG_INT_CONST_EXPR |	 \
+		CONSTEXPR_FLAG_INT_CONST |	 \
+		CONSTEXPR_FLAG_ENUM_CONST |	 \
+		CONSTEXPR_FLAG_CHAR_CONST)
+
+/*
  * Remove any "Constant" [6.4.4] flag, but retain the "constant
  * expression" [6.6] flags.
  */

new file mode 100644
index 0000000..2706961
@@ -0,0 +1,25 @@
+static int a[] = {
+	[(int)0] = 0,		// OK
+	[(int)(int)0] = 0,	// OK
+	[(int)0.] = 0,		// OK
+	[(int)(int)0.] = 0,	// OK
+	[(int)__builtin_choose_expr(0, 0, 0)] = 0,	// OK
+	[(int)__builtin_choose_expr(0, 0, 0.)] = 0,	// OK
+
+	[(int)(float)0] = 0,	// KO
+	[(int)(float)0.] = 0,	// KO
+
+	[(int)(void*)0] = 0,	// KO
+	[(int)(void*)0.] = 0,	// KO
+
+};
+/*
+ * check-name: Expression constness propagation in casts
+ *
+ * check-error-start
+constexpr-cast.c:9:11: error: bad integer constant expression
+constexpr-cast.c:10:11: error: bad integer constant expression
+constexpr-cast.c:12:11: error: bad integer constant expression
+constexpr-cast.c:13:11: error: bad integer constant expression
+ * check-error-end
+ */

2.7.0

Fine for me.

Feel free to add

Move the whole calculation of binary operations', compare and logical expressions' constness flags to the evaluation phase such that expressions like

  0 + __builtin_choose_expr(0, 0, 0)
  0 < __builtin_choose_expr(0, 0, 0)
  0 && __builtin_choose_expr(0, 0, 0)

can now be recognized as qualifying as integer constant expressions.

---
 evaluate.c                   | 28 +++++++++-------------------
 expression.c                 |  5 -----
 validation/constexpr-binop.c | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 24 deletions(-)
 create mode 100644 validation/constexpr-binop.c

index 24a7f2f..32e5930 100644
@@ -878,12 +878,9 @@ static struct symbol *evaluate_logical(struct expression *expr)
 
 	/* the result is int [6.5.13(3), 6.5.14(3)] */
 	expr->ctype = &int_ctype;
-	if (expr->constexpr_flags) {
-		if (!(expr->left->constexpr_flags &
-				expr->right->constexpr_flags &
-				CONSTEXPR_FLAG_INT_CONST_EXPR))
-			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
-	}
+	expr->constexpr_flags = expr->left->constexpr_flags &
+		expr->right->constexpr_flags &
+		~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 	return &int_ctype;
 }
 

@@ -894,15 +891,12 @@ static struct symbol *evaluate_binop(struct expression *expr)
 	int rclass = classify_type(expr->right->ctype, &rtype);
 	int op = expr->op;
 
-	if (expr->constexpr_flags) {
-		if (!(expr->left->constexpr_flags &
-				expr->right->constexpr_flags &
-				CONSTEXPR_FLAG_INT_CONST_EXPR))
-			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
-	}
-
 	/* number op number */
 	if (lclass & rclass & TYPE_NUM) {
+		expr->constexpr_flags = expr->left->constexpr_flags &
+			expr->right->constexpr_flags &
+			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
+
 		if ((lclass | rclass) & TYPE_FLOAT) {
 			switch (op) {
 			case '+': case '-': case '*': case '/':

@@ -1004,12 +998,8 @@ static struct symbol *evaluate_compare(struct expression *expr)
 	struct symbol *ctype;
 	const char *typediff;
 
-	if (expr->constexpr_flags) {
-		if (!(expr->left->constexpr_flags &
-				expr->right->constexpr_flags &
-				CONSTEXPR_FLAG_INT_CONST_EXPR))
-			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
-	}
+	expr->constexpr_flags = left->constexpr_flags &
+		right->constexpr_flags & ~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 
 	/* Type types? */
 	if (is_type_type(ltype) && is_type_type(rtype))

index 759bee8..5970b1b 100644
@@ -147,7 +147,6 @@ static struct token *builtin_types_compatible_p_expr(struct token *token,
 {
 	struct expression *expr = alloc_expression(
 		token->pos, EXPR_COMPARE);
-	expr->constexpr_flags = CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 	expr->op = SPECIAL_EQUAL;
 	token = token->next;
 	if (!match_op(token, '('))

@@ -766,10 +765,6 @@ static struct token *cast_expression(struct token *token, struct expression **tr
 				sparse_error(next->pos, "No right hand side of '%s'-expression", show_special(op));	\
 				break;					\
 			}						\
-			top->constexpr_flags =				\
-				left->constexpr_flags &		\
-				right->constexpr_flags &		\
-				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;	\
 			top->op = op;					\
 			top->left = left;				\
 			top->right = right;				\

new file mode 100644
index 0000000..85a88e3
@@ -0,0 +1,33 @@
+static int a[] = {
+	[0 + 0] = 0,						// OK
+	[0 + 0.] = 0,						// KO
+	[(void*)0 + 0] = 0,					// KO
+	[0 + __builtin_choose_expr(0, 0, 0)] = 0,		// OK
+	[0 + __builtin_choose_expr(0, 0., 0)] = 0,		// OK
+	[0 + __builtin_choose_expr(0, 0, 0.)] = 0,		// KO
+	[0 < 0] = 0,						// OK
+	[0 < 0.] = 0,						// KO
+	[0 < __builtin_choose_expr(0, 0, 0)] = 0,		// OK
+	[0 < __builtin_choose_expr(0, 0., 0)] = 0,		// OK
+	[0 < __builtin_choose_expr(0, 0, 0.)] = 0,		// KO
+	[0 && 0] = 0,						// OK
+	[0 && 0.] = 0,						// KO
+	[0 && __builtin_choose_expr(0, 0, 0)] = 0,		// OK
+	[0 && __builtin_choose_expr(0, 0., 0)] = 0,		// OK
+	[0 && __builtin_choose_expr(0, 0, 0.)] = 0,		// KO
+	[0 + __builtin_types_compatible_p(int, float)] = 0,	// OK
+};
+
+/*
+ * check-name: Expression constness propagation in binops and alike
+ *
+ * check-error-start
+constexpr-binop.c:3:12: error: bad constant expression
+constexpr-binop.c:4:19: error: bad integer constant expression
+constexpr-binop.c:7:12: error: bad constant expression
+constexpr-binop.c:9:12: error: bad integer constant expression
+constexpr-binop.c:12:12: error: bad integer constant expression
+constexpr-binop.c:14:12: error: bad integer constant expression
+constexpr-binop.c:17:12: error: bad integer constant expression
+ * check-error-end
+ */

2.7.0

Shouldn't it be better to already include the '~' into the definition of CONSTEXPR_FLAG_DECAY_CONSTS_MASK?

Otherwise it's fine for me.
Feel free to add

Move the whole calculation of prefix expressions' constness flags to the evaluation phase such that expressions like

-__builtin_choose_expr(0, 0, 0)
~__builtin_choose_expr(0, 0, 0)
!__builtin_choose_expr(0, 0, 0)

can now be recognized as qualifying as integer constant expressions.

---
 evaluate.c                   | 12 ++++++------
 expression.c                 |  5 -----
 validation/constexpr-preop.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 11 deletions(-)
 create mode 100644 validation/constexpr-preop.c

index 32e5930..43a0432 100644
@@ -1797,9 +1797,9 @@ static struct symbol *evaluate_sign(struct expression *expr)
 {
 	struct symbol *ctype = expr->unop->ctype;
 	int class = classify_type(ctype, &ctype);
-	if (expr->constexpr_flags && !(expr->unop->constexpr_flags &
-					CONSTEXPR_FLAG_INT_CONST_EXPR))
-		expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
+	unsigned char flags = expr->unop->constexpr_flags &
+		~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
+
 	/* should be an arithmetic type */
 	if (!(class & TYPE_NUM))
 		return bad_expr_type(expr);

@@ -1816,6 +1816,7 @@ Normal:
 	}
 	if (expr->op == '+')
 		*expr = *expr->unop;
+	expr->constexpr_flags = flags;
 	expr->ctype = ctype;
 	return ctype;
 Restr:

@@ -1853,9 +1854,8 @@ static struct symbol *evaluate_preop(struct expression *expr)
 		return evaluate_postop(expr);
 
 	case '!':
-		if (expr->constexpr_flags && !(expr->unop->constexpr_flags &
-					CONSTEXPR_FLAG_INT_CONST_EXPR))
-			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
+		expr->constexpr_flags = expr->unop->constexpr_flags &
+			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 		if (is_safe_type(ctype))
 			warning(expr->pos, "testing a 'safe expression'");
 		if (is_float_type(ctype)) {

index 5970b1b..302ef8b 100644
@@ -455,9 +455,6 @@ struct token *primary_expression(struct token *token, struct expression **tree)
 			expr = alloc_expression(token->pos, EXPR_PREOP);
 			expr->op = '(';
 			token = parens_expression(token, &expr->unop, "in expression");
-			if (expr->unop)
-				expr->constexpr_flags =
-					expr->unop->constexpr_flags;
 			break;
 		}
 		if (token->special == '[' && lookup_type(token->next)) {

@@ -670,8 +667,6 @@ static struct token *unary_expression(struct token *token, struct expression **t
 			unary = alloc_expression(token->pos, EXPR_PREOP);
 			unary->op = token->special;
 			unary->unop = unop;
-			unary->constexpr_flags = unop->constexpr_flags &
-				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 			*tree = unary;
 			return next;
 		}

new file mode 100644
index 0000000..5d869da
@@ -0,0 +1,29 @@
+static int a[] = {
+  [+0] = 0,					// OK
+  [+__builtin_choose_expr(0, 0, 0)] = 0,	// OK
+  [+0.] = 0,					// KO
+  [+__builtin_choose_expr(0, 0, 0.)] = 0,	// KO
+  [-0] = 0,					// OK
+  [-__builtin_choose_expr(0, 0, 0)] = 0,	// OK
+  [-0.] = 0,					// KO
+  [-__builtin_choose_expr(0, 0, 0.)] = 0,	// KO
+  [~0] = 0,					// OK
+  [~__builtin_choose_expr(0, 0, 0)] = 0,	// OK
+  [!0] = 0,					// OK
+  [!__builtin_choose_expr(0, 0, 0)] = 0,	// OK
+  [!0.] = 0,					// KO
+  [!__builtin_choose_expr(0, 0, 0.)] = 0,	// KO
+};
+
+/*
+ * check-name: Expression constness propagation in preops
+ *
+ * check-error-start
+constexpr-preop.c:4:5: error: bad constant expression
+constexpr-preop.c:5:33: error: bad constant expression
+constexpr-preop.c:8:4: error: bad constant expression
+constexpr-preop.c:9:4: error: bad constant expression
+constexpr-preop.c:14:4: error: bad integer constant expression
+constexpr-preop.c:15:4: error: bad integer constant expression
+ * check-error-end
+ */

2.7.0

Fien for me.

Feel free to add

Move the whole calculation of conditional expressions' constness flags to the evaluation phase such that expressions like

0 ? __builtin_choose_expr(0, 0, 0) : 0
0 ? 0 : __builtin_choose_expr(0, 0, 0)

can now be recognized as qualifying as integer constant expressions.

---
 evaluate.c                         | 12 ++++--------
 expression.c                       |  9 ---------
 validation/constexpr-conditional.c | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 17 deletions(-)
 create mode 100644 validation/constexpr-conditional.c

index 43a0432..03db2c3 100644
@@ -1115,14 +1115,10 @@ static struct symbol *evaluate_conditional_expression(struct expression *expr)
 		true = &expr->cond_true;
 	}
 
-	if (expr->constexpr_flags) {
-		int flags = (expr->conditional->constexpr_flags &
-			CONSTEXPR_FLAG_INT_CONST_EXPR);
-		flags &= (*true)->constexpr_flags &
-			expr->cond_false->constexpr_flags;
-		if (!flags)
-			expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
-	}
+	expr->constexpr_flags = (expr->conditional->constexpr_flags &
+				(*true)->constexpr_flags &
+				expr->cond_false->constexpr_flags &
+				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK);
 
 	lclass = classify_type(ltype, &ltype);
 	rclass = classify_type(rtype, &rtype);

index 302ef8b..b2d5eb4 100644
@@ -862,15 +862,6 @@ struct token *conditional_expression(struct token *token, struct expression **tr
 		token = parse_expression(token->next, &expr->cond_true);
 		token = expect(token, ':', "in conditional expression");
 		token = conditional_expression(token, &expr->cond_false);
-		if (expr->left && expr->cond_false) {
-			expr->constexpr_flags = expr->left->constexpr_flags &
-				expr->cond_false->constexpr_flags;
-			if (expr->cond_true)
-				expr->constexpr_flags &=
-					expr->cond_true->constexpr_flags;
-			expr->constexpr_flags &=
-				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
-		}
 	}
 	return token;
 }

new file mode 100644
index 0000000..a3331b3
@@ -0,0 +1,34 @@
+static int a[] = {
+	[0 ? : 0] = 0,						// OK
+	[1 ? : 0] = 0,						// OK
+	[0 ? 0 : 0] = 0,					// OK
+	[1 ? 0 : 0] = 0,					// OK
+	[0 ? 0 : __builtin_choose_expr(0, 0, 0)] = 0,		// OK
+	[1 ? __builtin_choose_expr(0, 0, 0) : 0] = 0,		// OK
+	[0 ? __builtin_choose_expr(0, 0, 0) : 0] = 0,		// OK
+	[1 ? 1 : __builtin_choose_expr(0, 0, 0)] = 0,		// OK
+	[__builtin_choose_expr(0, 0, 0) ? : 0] = 0,		// OK
+	[__builtin_choose_expr(0, 0, 1) ? : 0] = 0,		// OK
+	[0. ? : 0] = 0,					// KO
+	[0 ? 0. : 0] = 0,					// KO
+	[1 ? : 0.] = 0,					// KO
+	[__builtin_choose_expr(0, 0., 0) ? : 0] = 0,		// OK
+	[__builtin_choose_expr(0, 0, 0.) ? : 0] = 0,		// KO
+	[0 ? __builtin_choose_expr(0, 0., 0) : 0] = 0,		// OK
+	[0 ? __builtin_choose_expr(0, 0, 0.) : 0] = 0,		// KO
+	[1 ? 0 : __builtin_choose_expr(0, 0., 0)] = 0,		// OK
+	[1 ? 0 : __builtin_choose_expr(0, 0, 0.)] = 0,		// KO
+};
+
+/*
+ * check-name: Expression constness propagation in conditional expressions
+ *
+ * check-error-start
+constexpr-conditional.c:12:13: error: bad constant expression
+constexpr-conditional.c:13:19: error: bad constant expression
+constexpr-conditional.c:14:12: error: bad constant expression
+constexpr-conditional.c:16:42: error: bad constant expression
+constexpr-conditional.c:18:48: error: bad constant expression
+constexpr-conditional.c:20:14: error: bad constant expression
+ * check-error-end
+ */

2.7.0

Fine for me.

Feel free to add

Arithmetic constant expressions may be either of (6.6(8)):
- integer constant expressions
- floating point constants
or any arithmetic expression build up from them. Furthermore, casts with arithmetic destination types preserve arithmetic constness.

Arithmetic constant expressions may be used as initializers for objects of static storage duration.

Introduce a new constexpr_flag CONSTEXPR_FLAG_ARITH_CONST_EXPR.

Modify CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK and CONSTEXPR_FLAG_FP_CONST_SET_MASK to also include that new bit. Thus, whenever an integer constant expression or a floating point constant is recognized, it is automatically tagged as an arithmetic constant expression.

Note that everything has already been set up such that the new CONSTEXPR_FLAG_ARITH_CONST_EXPR flag propagates nicely from subexpressions to parent expressions at evaluation.

---
 expression.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

index 408382e..b85f1b1 100644
@@ -96,19 +96,23 @@ enum constexpr_flag {
 	/*
 	 * A constant expression in the sense of [6.6]:
 	 * - integer constant expression [6.6(6)]
+	 * - arithmetic constant expression [6.6(8)]
 	 */
 	CONSTEXPR_FLAG_INT_CONST_EXPR = (1 << 4),
+	CONSTEXPR_FLAG_ARITH_CONST_EXPR = (1 << 5),
 };
 
-#define CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK	\
-	(CONSTEXPR_FLAG_INT_CONST_EXPR)
+/* integer constant expression => arithmetic constant expression */
+#define CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK				\
+	(CONSTEXPR_FLAG_INT_CONST_EXPR | CONSTEXPR_FLAG_ARITH_CONST_EXPR)
 
 /* integer constant => integer constant expression */
 #define CONSTEXPR_FLAG_INT_CONST_SET_MASK			\
 	(CONSTEXPR_FLAG_INT_CONST | CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK)
 
-#define CONSTEXPR_FLAG_FP_CONST_SET_MASK	\
-	(CONSTEXPR_FLAG_FP_CONST)
+/* floating point constant => arithmetic constant expression */
+#define CONSTEXPR_FLAG_FP_CONST_SET_MASK				\
+	(CONSTEXPR_FLAG_FP_CONST | CONSTEXPR_FLAG_ARITH_CONST_EXPR)
 
 /* enumeration constant => integer constant expression */
 #define CONSTEXPR_FLAG_ENUM_CONST_SET_MASK				\

2.7.0

Fine for me.

Feel free to add

Address constants [6.6(9)] constitute one of the types of constant expressions allowed in initializers [6.6(7)] for static storage duration objects [6.7.8(4)].

Introduce a new flag CONSTEXPR_ADDR_CONSTANT for tagging expressions which qualify as being an address constant.

Make sure not to carry over the address constant attribute from subexpressions for operators that never yield address constants, i.e. most arithmetic ones, logical ones etc.

---
 evaluate.c   | 27 ++++++++++++++++++++++-----
 expression.h |  2 ++
 2 files changed, 24 insertions(+), 5 deletions(-)

index 03db2c3..dd44cd5 100644
@@ -880,7 +880,8 @@ static struct symbol *evaluate_logical(struct expression *expr)
 	expr->ctype = &int_ctype;
 	expr->constexpr_flags = expr->left->constexpr_flags &
 		expr->right->constexpr_flags &
-		~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
+		~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
+		~CONSTEXPR_FLAG_ADDR_CONST;
 	return &int_ctype;
 }
 

@@ -999,7 +1000,8 @@ static struct symbol *evaluate_compare(struct expression *expr)
 	const char *typediff;
 
 	expr->constexpr_flags = left->constexpr_flags &
-		right->constexpr_flags & ~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
+		right->constexpr_flags & ~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
+		~CONSTEXPR_FLAG_ADDR_CONST;
 
 	/* Type types? */
 	if (is_type_type(ltype) && is_type_type(rtype))

@@ -1115,10 +1117,15 @@ static struct symbol *evaluate_conditional_expression(struct expression *expr)
 		true = &expr->cond_true;
 	}
 
+	/*
+	 * A conditional operator never yields an address constant
+	 * [6.6(9)].
+	 */
 	expr->constexpr_flags = (expr->conditional->constexpr_flags &
 				(*true)->constexpr_flags &
 				expr->cond_false->constexpr_flags &
-				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK);
+				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
+				~CONSTEXPR_FLAG_ADDR_CONST);
 
 	lclass = classify_type(ltype, &ltype);
 	rclass = classify_type(rtype, &rtype);

@@ -1850,8 +1857,13 @@ static struct symbol *evaluate_preop(struct expression *expr)
 		return evaluate_postop(expr);
 
 	case '!':
+		/*
+		 * A logical negation never yields an address constant
+		 * [6.6(9)].
+		 */
 		expr->constexpr_flags = expr->unop->constexpr_flags &
-			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
+			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
+			~CONSTEXPR_FLAG_ADDR_CONST;
 		if (is_safe_type(ctype))
 			warning(expr->pos, "testing a 'safe expression'");
 		if (is_float_type(ctype)) {

@@ -2731,8 +2743,13 @@ static struct symbol *evaluate_cast(struct expression *expr)
 	class1 = classify_type(ctype, &t1);
 
 	if (class1 & TYPE_NUM) {
+		/*
+		 * Casts to numeric types never result in address
+		 * constants [6.6(9)].
+		 */
 		expr->constexpr_flags = target->constexpr_flags &
-			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
+			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
+			~CONSTEXPR_FLAG_ADDR_CONST;
 		/*
 		 * Cast to float type -> not an integer constant
 		 * expression [6.6(6)].

index b85f1b1..285c18c 100644
@@ -97,9 +97,11 @@ enum constexpr_flag {
 	 * A constant expression in the sense of [6.6]:
 	 * - integer constant expression [6.6(6)]
 	 * - arithmetic constant expression [6.6(8)]
+	 * - address constant [6.6(9)]
 	 */
 	CONSTEXPR_FLAG_INT_CONST_EXPR = (1 << 4),
 	CONSTEXPR_FLAG_ARITH_CONST_EXPR = (1 << 5),
+	CONSTEXPR_FLAG_ADDR_CONST = (1 << 6),
 };
 
 /* integer constant expression => arithmetic constant expression */

2.7.0

Fine for me.

Feel free to add

Initializers of static storage duration objects shall be constant expressions [6.7.8(4)].

Warn if that requirement is not met and the -Wstatic-initializer-not-const flag has been given on sparse's command line.

Identify static storage duration objects by having either of MOD_TOPLEVEL or MOD_STATIC set.

Check an initializer's constness at the lowest possible subobject level, i.e. at the level of the "assignment-expression" production in [6.7.8].

For compound objects, make handle_list_initializer() pass the surrounding object's storage duration modifiers down to handle_simple_initializer() at subobject initializer evaluation.

---
 evaluate.c                  | 26 +++++++++++++++++++-
 lib.c                       |  2 ++
 lib.h                       |  1 +
 sparse.1                    |  9 +++++++
 validation/constexpr-init.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 validation/constexpr-init.c

index dd44cd5..300bfbe 100644
@@ -2455,6 +2455,7 @@ static void handle_list_initializer(struct expression *expr,
 {
 	struct expression *e, *last = NULL, *top = NULL, *next;
 	int jumped = 0;
+	unsigned long old_modifiers;
 
 	FOR_EACH_PTR(expr->expr_list, e) {
 		struct expression **v;

@@ -2509,8 +2510,21 @@ found:
 		else
 			v = &top->ident_expression;
 
-		if (handle_simple_initializer(v, 1, lclass, top->ctype))
+		/*
+		 * Temporarily copy storage modifiers down from
+		 * surrounding type such that
+		 * handle_simple_initializer() can check
+		 * initializations of subobjects with static storage
+		 * duration.
+		 */
+		old_modifiers = top->ctype->ctype.modifiers;
+		top->ctype->ctype.modifiers =
+			old_modifiers | (ctype->ctype.modifiers & MOD_STORAGE);
+		if (handle_simple_initializer(v, 1, lclass, top->ctype)) {
+			top->ctype->ctype.modifiers = old_modifiers;
 			continue;
+		}
+		top->ctype->ctype.modifiers = old_modifiers;
 
 		if (!(lclass & TYPE_COMPOUND)) {
 			warning(e->pos, "bogus scalar initializer");

@@ -2620,6 +2634,16 @@ static int handle_simple_initializer(struct expression **ep, int nested,
 		if (!evaluate_expression(e))
 			return 1;
 		compatible_assignment_types(e, ctype, ep, "initializer");
+		/*
+		 * Initializers for static storage duration objects
+		 * shall be constant expressions or a string literal [6.7.8(4)].
+		 */
+		if ((ctype->ctype.modifiers & (MOD_TOPLEVEL | MOD_STATIC)) &&
+			!(e->constexpr_flags & (CONSTEXPR_FLAG_ARITH_CONST_EXPR
+					| CONSTEXPR_FLAG_ADDR_CONST)) &&
+			Wconstexpr_not_const)
+			warning(e->pos, "non-constant initializer for static object");
+
 		return 1;
 	}
 

index 8dc5bcf..75cea42 100644
@@ -219,6 +219,7 @@ int Waddress_space = 1;
 int Wbitwise = 0;
 int Wcast_to_as = 0;
 int Wcast_truncate = 1;
+int Wconstexpr_not_const = 0;
 int Wcontext = 1;
 int Wdecl = 1;
 int Wdeclarationafterstatement = -1;

@@ -442,6 +443,7 @@ static const struct warning {
 	{ "bitwise", &Wbitwise },
 	{ "cast-to-as", &Wcast_to_as },
 	{ "cast-truncate", &Wcast_truncate },
+	{ "constexpr-not-const", &Wconstexpr_not_const},
 	{ "context", &Wcontext },
 	{ "decl", &Wdecl },
 	{ "declaration-after-statement", &Wdeclarationafterstatement },

index 15b69fa..916eb31 100644
@@ -105,6 +105,7 @@ extern int Waddress_space;
 extern int Wbitwise;
 extern int Wcast_to_as;
 extern int Wcast_truncate;
+extern int Wconstexpr_not_const;
 extern int Wcontext;
 extern int Wdecl;
 extern int Wdeclarationafterstatement;

index 4adaf6c..7117bdf 100644
@@ -86,6 +86,15 @@ Sparse issues these warnings by default.  To turn them off, use
 \fB\-Wno\-cast\-truncate\fR.
 .
 .TP
+.B \-Wconstexpr-not-const
+Warn if a non-constant expression is encountered when really expecting a
+constant expression instead.
+Currently, this warns when initializing an object of static storage duration
+with an initializer which is not a constant expression.
+
+Sparse does not issue these warnings by default.
+.
+.TP
 .B \-Wcontext
 Warn about potential errors in synchronization or other delimited contexts.
 

2.7.0

"-Wstatic-initializer-not-const" s not what is used further in the code.

Also, I think it should be better to introduce this new -W flag in a separate patch.

Introduce support for recognizing address constants created either
- explicitly by referencing a static storage duration object by means of the unary & operator,
- implicitly by the use of an expression of array or function type.

Initially tag an expression as being an address constant at the primary expression level, i.e. upon encountering a symbol designating an object of static storage duration in primary_expression().

Carry the address constant flag over to the *-preop wrapped expression created by evaluate_symbol_expression().

When dereferencing such a *-preop wrapped expression, make evaluate_addressof() keep any address constant flag for the unwrapped expression.

---
 evaluate.c                            |  3 ++-
 expression.c                          |  8 ++++++++
 validation/constexpr-addr-of-static.c | 36 +++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 validation/constexpr-addr-of-static.c

index 300bfbe..91f89f4 100644
@@ -70,9 +70,11 @@ static struct symbol *evaluate_symbol_expression(struct expression *expr)
 	addr->symbol = sym;
 	addr->symbol_name = expr->symbol_name;
 	addr->ctype = &lazy_ptr_ctype;	/* Lazy evaluation: we need to do a proper job if somebody does &sym */
+	addr->constexpr_flags = expr->constexpr_flags;
 	expr->type = EXPR_PREOP;
 	expr->op = '*';
 	expr->unop = addr;
+	expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 
 	/* The type of a symbol is the symbol itself! */
 	expr->ctype = sym;

@@ -1682,7 +1684,6 @@ static struct symbol *evaluate_addressof(struct expression *expr)
 	}
 	ctype = op->ctype;
 	*expr = *op->unop;
-	expr->constexpr_flags = CONSTEXPR_FLAG_NONE;
 
 	if (expr->type == EXPR_SYMBOL) {
 		struct symbol *sym = expr->symbol;

index b2d5eb4..11fb9cd 100644
@@ -440,6 +440,14 @@ struct token *primary_expression(struct token *token, struct expression **tree)
 		}
 		expr->symbol_name = token->ident;
 		expr->symbol = sym;
+
+		/*
+		 * A pointer to an lvalue designating a static storage
+		 * duration object is an address constant [6.6(9)].
+		 */
+		if(sym && (sym->ctype.modifiers & (MOD_TOPLEVEL | MOD_STATIC)))
+			expr->constexpr_flags = CONSTEXPR_FLAG_ADDR_CONST;
+
 		token = next;
 		break;
 	}

new file mode 100644
index 0000000..a3af99a
@@ -0,0 +1,36 @@
+static int a = 1;
+static int b[2] = {1, 1};
+static void c(void) {}
+
+static int *d = &a;		// OK
+static int *e = d;		// KO
+static int *f = b;		// OK
+
+static void (*g)(void) = c;	// OK
+static void (*h)(void) = &c;	// OK
+
+static int *i = &*&a;		// OK
+static int *j = &*b;		// OK
+static int *k = &*d;		// KO
+
+
+static void l(void) {
+	int a = 1;
+	static int *b = &a;	// KO
+}
+
+static void m(void) {
+	static int a = 1;
+	static int *b = &a;	// OK
+}
+
+/*
+ * check-name: address of static object constness verification.
+ * check-command: sparse -Wconstexpr-not-const $file
+ *
+ * check-error-start
+constexpr-addr-of-static.c:6:17: warning: non-constant initializer for static object
+constexpr-addr-of-static.c:14:19: warning: non-constant initializer for static object
+constexpr-addr-of-static.c:19:26: warning: non-constant initializer for static object
+ * check-error-end
+ */

2.7.0

According to 6.6(9), an address constant may get created by casting an integer constant to pointer type.

Make evaluate_cast() handle this case, that is tag a cast expression as being an address constant if the target is a integer constant and the destination is of pointer type.

---
 evaluate.c                          | 10 +++++++++-
 validation/constexpr-pointer-cast.c | 13 +++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 validation/constexpr-pointer-cast.c

index 91f89f4..a740474 100644
@@ -2767,7 +2767,15 @@ static struct symbol *evaluate_cast(struct expression *expr)
 
 	class1 = classify_type(ctype, &t1);
 
-	if (class1 & TYPE_NUM) {
+	if (!(class1 & TYPE_NUM)) {
+		/*
+		 * Casts of integer literals to pointer type yield
+		 * address constants [6.6(9)].
+		 */
+		if (class1 & TYPE_PTR &&
+			(target->constexpr_flags & CONSTEXPR_FLAG_INT_CONST))
+			expr->constexpr_flags = CONSTEXPR_FLAG_ADDR_CONST;
+	} else {
 		/*
 		 * Casts to numeric types never result in address
 		 * constants [6.6(9)].

new file mode 100644
index 0000000..d19c108
@@ -0,0 +1,13 @@
+static int *a = (int*)0;	// OK
+static int b = 0;
+static int *c = (int*)b;	// KO
+
+
+/*
+ * check-name: integer literal cast to pointer type constness verification.
+ * check-command: sparse -Wconstexpr-not-const $file
+ *
+ * check-error-start
+constexpr-pointer-cast.c:3:18: warning: non-constant initializer for static object
+ * check-error-end
+ */

2.7.0

An address constant +/- an integer constant expression qualifies as an address constant again.
Furthermore, the array-subscript operator "[]" may be used in the creation of address constant.

Handle both cases by making evaluate_ptr_add() check whether an integer constant expression is added to an address constant and tag the result as being an address constant again if so.

---
 evaluate.c                           |  8 ++++++++
 validation/constexpr-pointer-arith.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 validation/constexpr-pointer-arith.c

index a740474..16b4ce0 100644
@@ -565,6 +565,14 @@ static struct symbol *evaluate_ptr_add(struct expression *expr, struct symbol *i
 	classify_type(degenerate(expr->left), &ctype);
 	base = examine_pointer_target(ctype);
 
+	/*
+	 * An address constant +/- an integer constant expression
+	 * yields an address constant again [6.6(7)].
+	 */
+	if ((expr->left->constexpr_flags & CONSTEXPR_FLAG_ADDR_CONST) &&
+		(expr->right->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR))
+		expr->constexpr_flags = CONSTEXPR_FLAG_ADDR_CONST;
+
 	if (!base) {
 		expression_error(expr, "missing type information");
 		return NULL;

new file mode 100644
index 0000000..a922028
@@ -0,0 +1,28 @@
+static int a = 1;
+static int b[2] = {1, 1};
+
+static int *c = &b[1];					// OK
+static int *d = (int*)0 + 1;				// OK
+static int *e = &b[1] + 1;				// OK
+static int *f = b + 1;					// OK
+static int *g = d + 1;					// KO
+static int *h = &a + 1;				// OK
+static int *i = &b[1] + 1;				// OK
+static int *j = b + 1;					// OK
+static int *k = d + 1;					// KO
+static int *l = &*&b[1];				// OK
+static int *m = &*(&a + 1);				// OK
+static int *n = &*(&b[1] + 1);				// OK
+static int *o = &*(b + 1);				// OK
+static int *p = &*(d + 1);				// KO
+
+/*
+ * check-name: pointer arithmetic constness verification.
+ * check-command: sparse -Wconstexpr-not-const $file
+ *
+ * check-error-start
+constexpr-pointer-arith.c:8:19: warning: non-constant initializer for static object
+constexpr-pointer-arith.c:12:19: warning: non-constant initializer for static object
+constexpr-pointer-arith.c:17:22: warning: non-constant initializer for static object
+ * check-error-end
+ */

2.7.0

Fine for me.

Feel free to add

According to 6.6(9), the member access operators "." and "->" may be used in the creation of address constants.

Uses of both operators amount to the creation of EXPR_DEREF expressions which are eventually fed into evaluate_offset() at evaluation.

Make evaluate_offset() propagate any address constant flag of the object containing the referenced member to the newly created pointer addition expression.

---
 evaluate.c                                   |  6 ++++++
 validation/constexpr-addr-of-static-member.c | 26 ++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 validation/constexpr-addr-of-static-member.c

index 16b4ce0..f31ba9c 100644
@@ -1957,6 +1957,12 @@ static struct expression *evaluate_offset(struct expression *expr, unsigned long
 	 * we ever take the address of this member dereference..
 	 */
 	add->ctype = &lazy_ptr_ctype;
+	/*
+	 * The resulting address of a member access through an address
+	 * constant is an address constant again [6.6(9)].
+	 */
+	add->constexpr_flags = expr->constexpr_flags;
+
 	return add;
 }
 

new file mode 100644
index 0000000..f944f21
@@ -0,0 +1,26 @@
+struct A {
+	int a;
+	int b[2];
+};
+
+struct B {
+	int c;
+	struct A d;
+};
+
+static struct B a= {1, {1, {1, 1}}};
+
+static int *b = &a.d.a;	// OK
+static int *c = &(&a.d)->a;	// OK
+static int *d = a.d.b;		// OK
+static int *e = (&a.d)->b;	// OK
+static int *f = &a.d.b[1];	// OK
+static int *g = &(&a.d)->b[1];	// OK
+
+/*
+ * check-name: address of static object's member constness verification.
+ * check-command: sparse -Wconstexpr-not-const $file
+ *
+ * check-error-start
+ * check-error-end
+ */

2.7.0

Fine for me.

Feel free to add

Introduce support for recognizing string literals as address constants.

Make evaluate_string() unconditionally tag the *-preop wrapped symbol expression as being an address constant.

---
 evaluate.c                    | 1 +
 validation/constexpr-string.c | 9 +++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 validation/constexpr-string.c

index f31ba9c..7a4af39 100644
@@ -108,6 +108,7 @@ static struct symbol *evaluate_string(struct expression *expr)
 	
 	addr->symbol = sym;
 	addr->ctype = &lazy_ptr_ctype;
+	addr->constexpr_flags = CONSTEXPR_FLAG_ADDR_CONST;
 
 	expr->type = EXPR_PREOP;
 	expr->op = '*';

new file mode 100644
index 0000000..e641a83
@@ -0,0 +1,9 @@
+static char *a = "foobar";	// OK
+
+/*
+ * check-name: string literal constness verification.
+ * check-command: sparse -Wconstexpr-not-const $file
+ *
+ * check-error-start
+ * check-error-end
+ */

2.7.0

Fine for me.

Feel free to add

As an extension, GCC allows labels to be referenced a la
label1:
...

  void *ptr = &&label1;

Tag these label references as being address constants allowing them to be used as initializers for objects of static storage duration.

---
 expression.c                    |  1 +
 validation/constexpr-labelref.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+)
 create mode 100644 validation/constexpr-labelref.c

index 11fb9cd..ac6cf43 100644
@@ -687,6 +687,7 @@ static struct token *unary_expression(struct token *token, struct expression **t
 				sym->ctype.modifiers |= MOD_ADDRESSABLE;
 				add_symbol(&function_computed_target_list, sym);
 			}
+			label->constexpr_flags = CONSTEXPR_FLAG_ADDR_CONST;
 			label->label_symbol = sym;
 			*tree = label;
 			return token->next->next;

new file mode 100644
index 0000000..c51f9c8
@@ -0,0 +1,15 @@
+static void a(void)
+{
+label1:
+	;
+	static void *b = &&label1;
+}
+
+/*
+ * check-name: label reference constness verification.
+ * check-command: sparse -Wconstexpr-not-const $file
+ *
+ * check-error-start
+ * check-error-end
+ */
+

2.7.0

Fine for me.

Feel free to add

Currently, the determination of a __builtin_offsetof() expressions' constness flags is done in two steps:
- Several flags are speculatively set at expression parsing time
- and possibly cleared again at evaluation if the member expression includes a non-const array index like in __builtin_offsetof(struct A, a.b[non_const_foo])

For consistency with other expression types' evaluation, defer the determination of a __builtin_offsetof() expression's constness to evaluation time, too.

Furthermore, carry an array index expression's constness flags through the implicit cast to size_t type.

---
 evaluate.c                      | 10 +++++++---
 expression.c                    |  6 ------
 validation/constexpr-offsetof.c | 21 +++++++++++++++++++++
 3 files changed, 28 insertions(+), 9 deletions(-)
 create mode 100644 validation/constexpr-offsetof.c

index 7a4af39..0101e61 100644
@@ -3031,25 +3031,29 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
 		} else {
 			struct expression *idx = expr->index, *m;
 			struct symbol *i_type = evaluate_expression(idx);
+			unsigned old_idx_flags;
 			int i_class = classify_type(i_type, &i_type);
+
 			if (!is_int(i_class)) {
 				expression_error(expr, "non-integer index");
 				return NULL;
 			}
 			unrestrict(idx, i_class, &i_type);
+			old_idx_flags = idx->constexpr_flags;
 			idx = cast_to(idx, size_t_ctype);
+			idx->constexpr_flags = old_idx_flags;
 			m = alloc_const_expression(expr->pos,
 						   bits_to_bytes(ctype->bit_size));
 			m->ctype = size_t_ctype;
-			m->constexpr_flags |=
-				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
+			m->constexpr_flags = CONSTEXPR_FLAG_INT_CONST_SET_MASK;
 			expr->type = EXPR_BINOP;
 			expr->left = idx;
 			expr->right = m;
 			expr->op = '*';
 			expr->ctype = size_t_ctype;
 			expr->constexpr_flags = m->constexpr_flags &
-				idx->constexpr_flags;
+				idx->constexpr_flags &
+				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 		}
 	}
 	if (e) {

index ac6cf43..c3d54fe 100644
@@ -200,8 +200,6 @@ static struct token *builtin_offsetof_expr(struct token *token,
 			return expect(token, ')', "at end of __builtin_offset");
 		case SPECIAL_DEREFERENCE:
 			e = alloc_expression(token->pos, EXPR_OFFSETOF);
-			e->constexpr_flags =
-				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			e->op = '[';
 			*p = e;
 			p = &e->down;

@@ -209,8 +207,6 @@ static struct token *builtin_offsetof_expr(struct token *token,
 		case '.':
 			token = token->next;
 			e = alloc_expression(token->pos, EXPR_OFFSETOF);
-			e->constexpr_flags =
-				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			e->op = '.';
 			if (token_type(token) != TOKEN_IDENT) {
 				sparse_error(token->pos, "Expected member name");

@@ -222,8 +218,6 @@ static struct token *builtin_offsetof_expr(struct token *token,
 		case '[':
 			token = token->next;
 			e = alloc_expression(token->pos, EXPR_OFFSETOF);
-			e->constexpr_flags =
-				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			e->op = '[';
 			token = parse_expression(token, &e->index);
 			token = expect(token, ']',

new file mode 100644
index 0000000..d1697b0
@@ -0,0 +1,21 @@
+struct A {
+	int a[1];
+	int b;
+};
+
+extern int c;
+
+static int o[] = {
+	[__builtin_offsetof(struct A, b)] = 0,		// OK
+	[__builtin_offsetof(struct A, a[0])] = 0,	// OK
+	[__builtin_offsetof(struct A, a[0*0])] = 0,	// OK
+	[__builtin_offsetof(struct A, a[c])] = 0	// KO
+};
+
+/*
+ * check-name: __builtin_offsetof() constness verification.
+ *
+ * check-error-start
+constexpr-offsetof.c:12:39: error: bad constant expression
+ * check-error-end
+ */

2.7.0

I don't think that consistency is the good/real reason, you're doing it for the same reason as the others: so that the expression is recognized as an integer constant expression.

Unconditionally flag the expressions
__builtin_constant_p(),
__builtin_safe_p(),
__builtin_warning()
as being integer constant expressions.

---
 symbol.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

index 0ceff62..42e6a8f 100644
@@ -642,9 +642,10 @@ struct symbol *create_symbol(int stream, const char *name, int type, int namespa
 	return sym;
 }
 
-static int evaluate_to_integer(struct expression *expr)
+static int evaluate_to_int_const_expr(struct expression *expr)
 {
 	expr->ctype = &int_ctype;
+	expr->constexpr_flags |= CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 	return 1;
 }
 

@@ -749,17 +750,17 @@ out:
 }
 
 static struct symbol_op constant_p_op = {
-	.evaluate = evaluate_to_integer,
+	.evaluate = evaluate_to_int_const_expr,
 	.expand = expand_constant_p
 };
 
 static struct symbol_op safe_p_op = {
-	.evaluate = evaluate_to_integer,
+	.evaluate = evaluate_to_int_const_expr,
 	.expand = expand_safe_p
 };
 
 static struct symbol_op warning_op = {
-	.evaluate = evaluate_to_integer,
+	.evaluate = evaluate_to_int_const_expr,
 	.expand = expand_warning
 };
 

2.7.0

Fine for me.

Feel free to add

The official constraints on constant expressions [6.6] are insanely strict in that they do not allow some constructs commonly used in the wild.

Relax them by treating
- address constants cast to different pointer type as address constants again,
- address constants cast to arithmetic type as arithmetic constant expressions
- conditional expressions whose true and false branches both yield address constants as address constants,
- and conditional expressions whose condition is an address constant as an constant expression to the extent their true and false branches allow.

---
 evaluate.c | 38 ++++++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

index 0101e61..ff51d84 100644
@@ -1129,14 +1129,23 @@ static struct symbol *evaluate_conditional_expression(struct expression *expr)
 	}
 
 	/*
+	 * A conditional operator yields a particular constant
+	 * expression type only if all of its three subexpressions are
+	 * of that type [6.6(6), 6.6(8)].
+	 * As an extension, relax this restriction by allowing any
+	 * constant expression type for the condition expression.
+	 *
 	 * A conditional operator never yields an address constant
 	 * [6.6(9)].
+	 * However, as an extension, if the condition is any constant
+	 * expression, and the true and false expressions are both
+	 * address constants, mark the result as an address constant.
 	 */
-	expr->constexpr_flags = (expr->conditional->constexpr_flags &
-				(*true)->constexpr_flags &
-				expr->cond_false->constexpr_flags &
-				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
-				~CONSTEXPR_FLAG_ADDR_CONST);
+	if (expr->conditional->constexpr_flags &
+		(CONSTEXPR_FLAG_ARITH_CONST_EXPR | CONSTEXPR_FLAG_ADDR_CONST))
+		expr->constexpr_flags = (*true)->constexpr_flags &
+			expr->cond_false->constexpr_flags &
+			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 
 	lclass = classify_type(ltype, &ltype);
 	rclass = classify_type(rtype, &rtype);

@@ -2786,9 +2795,18 @@ static struct symbol *evaluate_cast(struct expression *expr)
 		/*
 		 * Casts of integer literals to pointer type yield
 		 * address constants [6.6(9)].
+		 *
+		 * As an extension, treat address constants cast to a
+		 * different pointer type as address constants again.
+		 *
+		 * As another extension, treat integer constant
+		 * expressions (in contrast to literals) cast to
+		 * pointer type as address constants.
 		 */
 		if (class1 & TYPE_PTR &&
-			(target->constexpr_flags & CONSTEXPR_FLAG_INT_CONST))
+			(target->constexpr_flags &
+				(CONSTEXPR_FLAG_INT_CONST_EXPR |
+					CONSTEXPR_FLAG_ADDR_CONST)))
 			expr->constexpr_flags = CONSTEXPR_FLAG_ADDR_CONST;
 	} else {
 		/*

@@ -2798,6 +2816,14 @@ static struct symbol *evaluate_cast(struct expression *expr)
 		expr->constexpr_flags = target->constexpr_flags &
 			~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
 			~CONSTEXPR_FLAG_ADDR_CONST;
+
+		/*
+		 * As an extension, treat address constants cast to
+		 * integer type as an arithmetic constant.
+		 */
+		if (target->constexpr_flags & CONSTEXPR_FLAG_ADDR_CONST)
+			expr->constexpr_flags = CONSTEXPR_FLAG_ARITH_CONST_EXPR;
+
 		/*
 		 * Cast to float type -> not an integer constant
 		 * expression [6.6(6)].

2.7.0

Fine for me.

Feel free to add

Sorry that wasn't for this patch

...

Toplevel compound literals have got static storage duration [6.5.2.5(6)].

This implies that
1. their addresses are address constants [6.6(9)] and
2. their initializers must contain constant expressions only [6.5.2.5(3), 6.7.8(4)] .

Flag the anonymous symbol created at expression parsing time as having static storage duration if the compound literal occurs at top level scope.

Flag the whole expression as being an address constant at evaluation time if its corresponding anonymous symbol had been previously marked as having static storage duration.

---
 evaluate.c                              |  2 ++
 expression.c                            |  2 ++
 validation/constexpr-compound-literal.c | 19 +++++++++++++++++++
 3 files changed, 23 insertions(+)
 create mode 100644 validation/constexpr-compound-literal.c

index ff51d84..06c360f 100644
@@ -2773,6 +2773,8 @@ static struct symbol *evaluate_cast(struct expression *expr)
 
 		addr->ctype = &lazy_ptr_ctype;	/* Lazy eval */
 		addr->symbol = sym;
+		if (sym->ctype.modifiers & MOD_TOPLEVEL)
+			addr->constexpr_flags |= CONSTEXPR_FLAG_ADDR_CONST;
 
 		expr->type = EXPR_PREOP;
 		expr->op = '*';

index c3d54fe..f49c8f6 100644
@@ -714,6 +714,8 @@ static struct token *cast_expression(struct token *token, struct expression **tr
 			cast->cast_type = sym;
 			token = expect(token, ')', "at end of cast operator");
 			if (match_op(token, '{')) {
+				if (toplevel(block_scope))
+					sym->ctype.modifiers |= MOD_TOPLEVEL;
 				if (is_force)
 					warning(sym->pos,
 						"[force] in compound literal");

new file mode 100644
index 0000000..d7f21ad
@@ -0,0 +1,19 @@
+static int *a = &(int){ 1 };	// OK
+static int *b = &(int){ *a };	// KO
+
+static void foo(void)
+{
+	int *b = &(int){ 1 };		// OK
+	int *c = &(int){ *a };		// OK
+	static int *d = &(int){ 1 };	// KO
+}
+
+/*
+ * check-name: compound literal address constness verification
+ * check-command: sparse -Wconstexpr-not-const $file
+ *
+ * check-error-start
+constexpr-compound-literal.c:2:25: warning: non-constant initializer for static object
+constexpr-compound-literal.c:8:27: warning: non-constant initializer for static object
+ * check-error-end
+ */

2.7.0

...

Consider the following code snippet:
static inline foo(int dummy, ...) {}
static int a = 0;
static void bar(void)
{
foo(0, a);
}

Sparse moans:
test.c:5:9: warning: initializer for static storage duration object
is not a constant expression

The cause can be tracked down as follows:
The anonymous node created by inline_function() for the variadic argument will get assigned to its base_type whatever the passed expression's ctype is. For the special case of a primary expression referencing a symbol, this ctype is the referenced symbol itself. Furthermore, inline_function() sets that symbol node's initializer to this expression.

Now, when the anonymous symbol node is evaluated, its base_type is handled in examine_base_type(). This applies the base_type's modifiers, i.e. the referenced symbol's MOD_STATIC in this case, to the inheriting ctype, that of the anonymous node, itself.
This in turn instructs the evaluation of the symbol's initializer to allow constant expressions only.

Do not inherit a base_type's storage related modifiers in examine_base_type().

---
 symbol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 42e6a8f..4766adb 100644
@@ -214,7 +214,8 @@ static struct symbol *examine_base_type(struct symbol *sym)
 	if (!base_type || base_type->type == SYM_PTR)
 		return base_type;
 	sym->ctype.as |= base_type->ctype.as;
-	sym->ctype.modifiers |= base_type->ctype.modifiers & MOD_PTRINHERIT;
+	sym->ctype.modifiers |= base_type->ctype.modifiers & MOD_PTRINHERIT &
+		~MOD_STORAGE;
 	concat_ptr_list((struct ptr_list *)base_type->ctype.contexts,
 			(struct ptr_list **)&sym->ctype.contexts);
 	if (base_type->type == SYM_NODE) {

2.7.0

The expression parsing code builds an EXPR_COMPARE expression around two EXPR_TYPE expressions for __builtin_types_compatible_p().

The EXPR_TYPE expressions are tagged as being integer constant expressions in order to trick the generic comparison evaluation code into flagging the result as an integer constant expression again.

Avoid this trickery by making evaluate_compare() unconditionally tag a comparsion between types as an integer constant expression.

---
 evaluate.c                                | 16 +++++++++++-----
 expression.c                              |  5 -----
 validation/constexpr-types-compatible-p.c |  9 +++++++++
 3 files changed, 20 insertions(+), 10 deletions(-)
 create mode 100644 validation/constexpr-types-compatible-p.c

index 06c360f..31ac4e1 100644
@@ -1010,17 +1010,23 @@ static struct symbol *evaluate_compare(struct expression *expr)
 	struct symbol *ctype;
 	const char *typediff;
 
-	expr->constexpr_flags = left->constexpr_flags &
-		right->constexpr_flags & ~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
-		~CONSTEXPR_FLAG_ADDR_CONST;
-
 	/* Type types? */
-	if (is_type_type(ltype) && is_type_type(rtype))
+	if (is_type_type(ltype) && is_type_type(rtype)) {
+		/*
+		 * __builtin_types_compatible_p() yields an integer
+		 * constant expression
+		 */
+		expr->constexpr_flags = CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 		goto OK;
+	}
 
 	if (is_safe_type(left->ctype) || is_safe_type(right->ctype))
 		warning(expr->pos, "testing a 'safe expression'");
 
+	expr->constexpr_flags = left->constexpr_flags &
+		right->constexpr_flags & ~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
+		~CONSTEXPR_FLAG_ADDR_CONST;
+
 	/* number on number */
 	if (lclass & rclass & TYPE_NUM) {
 		ctype = usual_conversions(expr->op, expr->left, expr->right,

index f49c8f6..4285cc8 100644
@@ -131,8 +131,6 @@ static struct token *parse_type(struct token *token, struct expression **tree)
 {
 	struct symbol *sym;
 	*tree = alloc_expression(token->pos, EXPR_TYPE);
-	(*tree)->constexpr_flags =
-		CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK; /* sic */
 	token = typename(token, &sym, NULL);
 	if (sym->ident)
 		sparse_error(token->pos,

@@ -461,9 +459,6 @@ struct token *primary_expression(struct token *token, struct expression **tree)
 		}
 		if (token->special == '[' && lookup_type(token->next)) {
 			expr = alloc_expression(token->pos, EXPR_TYPE);
-			/* sic */
-			expr->constexpr_flags =
-				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			token = typename(token->next, &expr->symbol, NULL);
 			token = expect(token, ']', "in type expression");
 			break;

new file mode 100644
index 0000000..78c37b4
@@ -0,0 +1,9 @@
+static int a[] = {[__builtin_types_compatible_p(int, int)] = 0};
+
+/*
+ * check-name: __builtin_types_compatible_p() constness verification.
+ *
+ * check-error-start
+ * check-error-end
+ */
+

2.7.0

Fine for me.

Feel free to add

Just a gently push to get some reviews on this...

Sorry for the late reply.

I take a look of your V3 patches.

May I ask a few questions regarding the constant expression.

+       CONSTEXPR_FLAG_INT_CONST = (1 << 0),
+       CONSTEXPR_FLAG_FP_CONST = (1 << 1),
+       CONSTEXPR_FLAG_ENUM_CONST = (1 << 2),
+       CONSTEXPR_FLAG_CHAR_CONST = (1 << 3),

Can I say each of the above constant type are elusive to each other? e.g. the floating point constant can not be a integer constant at the same time.

+
+ /*

+        * A constant expression in the sense of [6.6]:
+        * - integer constant expression [6.6(6)]
+        */

+ CONSTEXPR_FLAG_INT_CONST_EXPR = (1 << 4),

Can we express the const expression in terms of above constant flags? Each expression will have a ctype associate with it. It can be one of the int/fp/enum/char type.

e.g. "1.0 + 1" is a floating type expression according to the C rules.

In other words, it seems to me that the constant expression should have a deterministic ctype. We should be able to reuse the above constant flag without adding a new one. If not, please give some example to help me understand the issue.

I am not suggesting to change your patches at this stage. It just help me understand your patch.

Thanks

Chris

Hi Chris,

no problem, I just wanted to make sure that this series doesn't orphan in the end.

Yes, that's correct, they are all exclusive to each other. To make it explicit: the above flags apply to literals.

Yes, it is an arithmetic constant expression of floating point type.

Just like (int)(1.0 + 1) is an arithmetic constant expression of integer type, but *not* and integer constant expression: floating constants are allowed in integer constant expressions only if they are the *immediate* operands of casts [6.6(6)].

I.
In the first place, we really need to distinguish between the higher level constant expression productions
- integer constant expressions
- arithmetic constant expressions
- address constants

Example:
int a[] = { [(int)(0.0 + 0)] = 0 };

is not valid C code, since (int)(0.0 + 0) is not an integer constant expression (see above).

OTOH, (int)0.0 *is* an integer constant expression.

Thus, it is not enough to tag floating constants as simply being an arithmetic constant expression, the distinction is really needed here.

II.
For the case of integer constant literals vs. integer constant
expressions: in theory, the distinction between integer constant literals and enum/char constants is necessary, because

(void*)0

qualifies as an address constant, but

  enum foo { bar = 0, };
  (void*)bar

does not [6.6(9)].

To be honest, this overly strict requirement on address constants is relaxed in
[18/21] ("evaluate: relax some constant expression rules for pointer expressions")
again. However, this relaxation allows for constructs such as
(void *)(int)0.0
qualifying as an address constant which might not be the desired behaviour.

Conclusion: As it stands, we should be able to do
#define CONSTEXPR_FLAG_INT_CONST CONSTEXPR_FLAG_INT_CONST_EXPR #define CONSTEXPR_FLAG_ENUM_CONST CONSTEXPR_FLAG_INT_CONST_EXPR #define CONSTEXPR_FLAG_CHAR_CONST CONSTEXPR_FLAG_INT_CONST_EXPR
since we do not distinguish between integer constant expressions and integer constant literals. However this ignorance violates the C standard, in particular [6.6(9)] and we might want to be able to easily change this in the future. And yes, we could certainly distinguish between a char/enum constant and an integer constant literal based on its ctype. However, I personally don't like to treat integer, enum and char consts different than floating point consts, since they are kind of at the same level logically.

Thank you for reviewing!

Nicolai

Sorry for haven't been able to comment on this earlier. I'm catching up.

Regards,
Luc Van Oostenryck

I had a few remarks and some questions in some of the patches but I think it's turning pretty good.

I'm wondering if you have already tried to use it on something of interesting size like the kernel? If yes, did it gave something useful?

Chris, I have reviewed this rather deeply. I think the approach is sound and sane et the code seems quite correct. Do you think you will have some time soon to look at it?

Regards,
Luc

Chris, is it possible to have some feedback on this, please?

Is there anything more I can do to help?

Best regards,

Luc

OK, I am taking a stab at his again. Sorry for the long hold off. I will reply from this thread instead of finding the right patch email to reply on.

+++ validation/constexpr-cast.c 2016-11-23 09:57:37.638646787 +0800 @@ -0,0 +1,25 @@
+static int a[] = {

+       [(int)0] = 0,           // OK
+       [(int)(int)0] = 0,      // OK
+       [(int)0.] = 0,          // OK
+       [(int)(int)0.] = 0,     // OK
+       [(int)__builtin_choose_expr(0, 0, 0)] = 0,      // OK
+       [(int)__builtin_choose_expr(0, 0, 0.)] = 0,     // OK

+
+ [(int)(float)0] = 0, // KO

Why is this one KO? It seems perfectly fine to force a floating type into a integer constant expression.

per [6.6.6] quote: on integer constant expression:
" and floating constants that are the
immediate operands of casts. Cast operators in an integer constant expression shall only
convert arithmetic types to integer types,"

+ [(int)(float)0.] = 0, // KO
+
+ [(int)(void*)0] = 0, // KO

Also this one should be fine?

[6.7.8.6] quote as
" If a designator has the form
[ constant-expression ]
then the current object (defined below) shall have array type and the expression shall be
an *integer constant expression*."

So it seems the only requirement is *integer constant expression*. why (int)(void*)0 is not integer constant expression? Sure there is a warning about converting the (void*) into (int) for a different size. But that is a separate issue.

Chris

But, quoting the beginning of 6.6/6:
An integer constant expression shall have integer type and shall only have operands that are integer constants, enumeration constants, character constants, sizeof expressions whose results are integer constants, and **floating constants** that are the immediate operands of casts.

and the part '(float) 0' is none of these.

same for '(void *) 0'.

It's obvious for me and you that those once casted to (int) should evaluate to '0' and thus are constant expression but the strict interpretation of the standard say something else.

And to be honest, I find ridiculous that '(int) 0.0' is a integer constant expression and '(int)(float)0' is not.

IIRC, Nicolai had foreseen to relax those if needed, possibly even in one of the later patch or to introduce a '-Wstrict' option or something like this.

Luc

I see, thanks for pointing it out.

Good to know

Another question:
Is there any real reason to distinguish between "const enum/char" and "const integer"?
Can we roll "const enum/char" as one type of "const integer" as well?

I think the patch can be simpler. The dance of CONSTEXPR_SET and MASK etc is really messy.

The way I see it, there are three basic type of const elements. Int ( including enum/char), Float, Address.

Either of which does not has any overlap with each other.

Then each of the matching requirement is just a sets consist of the above element.
I am not sure there is need to distinguish integer const expression vs arthritic const expression. (Example please?) If it does, add one more bit as arthritic operations.

I think the real messy part of the original patch is that, CONSTEXPR_FLAG_INT_CONST_EXPR
and CONSTEXPR_FLAG_ARITH_CONST_EXPR are two separate bit need to be maintained. It does not need to be if we treat them as set of the elements.

We only need to keep track of "Int_const_expr, Float_literal, Addr_const_expr" three bits. May be Arthritic_const_expr if we have need for that. This patch series really make a difference on adding the "Addr_const_expr".

I am working on a patch on top of the patch series to test it out.

Chris

Hi Christopher,

(void*)'a' doesn't qualify as an address constant, (void *)0 does.

So while we certainly won't need to distinguish enum and char constants (literals), we have to do so for integer constants (literals) vs enum/char constants (literals).

Maybe it suffices to simply tag the latter as "integer constant expression" though. I will have a look on whether the standard allows character/enum constants at places where it forbids the use of integer constant expressions.

Sorry, I don't understand that last sentence. "Set" in the mathematical sense? How would that look like?

An example would be initialization with designators as shown in the testcase you quoted above. C.f. 6.7.8(6): only integer constant expressions are allowed there, but not arithmetic ones.

Prove me wrong, but AFAICS, in the context of error catching, the standard mandates that we need to distinguish between

- integer literals
- (maybe char/enum integer constants)
- floating point literals
- integer constant expressions
- arithmetic constant expressions
- address constants

somehow...

Thanks,

Nicolai

Good. That is exactly the corner case condition I am looking for. (void*)'a' is not an address constant, is it still consider constant expression?

How about (void*)(0 + 'a') and (void*)0 + 'a', are they address constant or not?

So I assume (void*) Enum_member is not address constant either, right?

Yes, "Set" in the mathematical sense. Maybe it is easier to show the code. I will try to explain here.

Basically, there are six bits as basic element. One bit per element. Integer constant, enum constant, char constant, float constant, address constant,
arithmetic constant.

The integer const expression can be test as a function, which use set operation base on the previous six bits. e.g. is_int_const_expr(flag) (flag & (Int | Enum | Char)) && !(flag & (Float | Addr | Arith)))

In other words, the is_int_const_expr() can be deduce from the six element bits. It does not need to maintain and propagate as a separate bit in the expr->flags.
We can calculate that result when it is needed.

Maintain and propagate the six const bits become easier because it does not need to
re-adjust the CONSTEXPR_FLAG_INT_CONST_EXPR bit all the time. At least that is what I am hoping for. We will see if this can work out or not.

Good, another corner case condition. That meas the arithmetic bits is here to stay. BTW, your test case is very good.

Yes, we can still distinguish them, no disagreement there. I just try to make the internal representation cleaner. I will find out more soon.

Chris

I don't think that this would qualify as a constant expression of any type.

Strictly speaking, neither of them is.

Despite of this, we would (hopefully) mark the second case as an address constant though.

Reason: this series flags address constants as such because they may be used as a "constant expression in an initializer", i.e. as initializers for static storage duration variables.

C.f. 6.6(7).

This paragraphs says that "an address constant for an object type plus or minus an integer constant expression" is allowed as a constant expression in an initializer.

Thus, after this series, sparse would tag (void*)0 + 'a' as being an address constant although it's really an address constant plus/minus an integer constant expression.

Right. The rule reads as "integer constant cast to pointer type", not "integer constant *expression* cast to pointer type".

For the same reason, (void*)(0 + 'a') isn't an address constant either.

+ integer constant *expression*

Take 0 + 0, for example: this is not an integer constant, but an integer constant *expression* (and an arithmetic constant expression as well).

Thanks :)

Nicolai

That is good info. Thanks for the clarify.

BTW, gcc does not seem to complain about const integer issue on,

         [(int)(float)0] = 0,    // KO
         [(int)(void*)0] = 0,    // KO

gcc seems does what make snese rather than stick to the book.

In your patch:

static struct token *builtin_types_compatible_p_expr(struct token *token,
struct expression **tree)

 {
        struct expression *expr = alloc_expression(
                token->pos, EXPR_COMPARE);
-       expr->flags = Int_const_expr;
        expr->op = SPECIAL_EQUAL;
        token = token->next;
        if (!match_op(token, '('))

I think __builtin_xxx function expand similar to the processor macro. In that sense, we might want to the consider this expression as integer constant express as if the __builtin_xxx call is either 0 or 1.

Again, gcc does not complain about:

       [__builtin_types_compatible_p(typeof(1), int)] = 0,   // KO

This makes sense, but not according the "strict" rules.

Ah, I see. I change my plan according this. The six basic element define as:
Integer constant, enum constant, char constant, float constant, address constant, composite op.

The composite op bit get set after binary operation or uniop. The flags can have more than one bit set for the expression.

Integer constant expression can be tested as:

!(flags & ( Addr | Float) ) && flag

Arithmetic constant expression can be tested as:

!(flags & Addr) ) && flag

Do you see any problem with this internal representation?

Chris

Never mind, I think you move it to the evaluation phase.

Chris

To give you some background: when I started this, my initial intent was to move the constness inspection from the expression building stage in expression.c to their evaluation in evaluation.c. This would be needed in order to decide whether the outcome of __builtin_choose_expr() is a constant expression, for example.

While doing this, I recognized that sparse's current constant expression tagging scheme was quite limited and I had to touch everything related to it anyway. So I asked on this list on whether it would be a good thing to let sparse be more strict than gcc in this regard and the feedback was that it certainly would (at user option).

Can you tell which one? If not, would resending this series help?

After [21/21] ("evaluation: treat comparsions between types as integer constexpr"), sparse should treat __builtin_types_compatible_p() as an integer constant expression, just as gcc does.

After all, the standard doesn't say anything about __builtin_types_compatible_p()...

So, if the above testcase really fails after this series, it's either a bug or an artifact of having this typeof() in there (or both), I think.

(int)0.0 is an integer constant expression.

In your scheme, it would have "composite op" and "float" set? The integer constant expression test you proposed above would fail in this case.

Thanks,

Nicolai

Good. Thanks for reminding of that. So ideally this is under a more strict options.

Yes, I can. But never mind that point as I reply in previous email. I figure you do the same work in the evaluation stage now.

No, please don't send the series again. I have a script system to process the patches now. So no patch will be missed from now on.

Never mind that.

(int) 0.0 will express as "Composite op" and "Int const" set. Cast is a special operation it will strip the "float" and convert it to "int". After all that is what cast does.

It will not change your patch behavior and warning etc, it is just a internal representation difference. How the constant expression bits are store and interpreted.

Chris

As it stands, [09/21] ("evaluate: check static storage duration objects' intializers' constness"), introduces the opt-in -Wconstexpr-not-const. This affects only initializers for objects of static storage duration.

So, there is currently no way to opt out from the stricter integer constant expression checks etc. introduced with this series.

I ran sparse with this series applied on the kernel back then and if I remember correctly, there was no spectacular difference though.

If these stricter semantics nevertheless became a problem, this would be easily "fixable" by further relaxing them at user option, I think.

Alright. May I further ask whether you've got all these precious Reviewed-by tags from Luc carried over? He did a great amount of review work on this series and it would be quite sad if these got lost. I still have them, so just tell me if you need a list. If not:
nevermind.

Ah ok, i mistook you in the semantics of the composite op flag.

Yes, yes got that. Don't get me wrong, I'm not defending my particular choice of flags. If yours leads to cleaner/less code, I will be very happy.

That being said, here's the next "corner case":

0 + 'a'

Which flags would be set for this binary expression? All of "composite op", "integer const", "char const"?

That would work with your test for integer constant expressions.

I think, it wouldn't be enough to just set "composite op" flag w/o anything else since the above integer constant expression needs to get distinguished from

0 + 0.0

which is an arithmetic constant expression only. Thus, considering your tests for integer vs. arithmetic constant expressions, this would need to have at least its "float constant" flag set?

Thank you,

Nicolai

That is fine for now. We can always add the option later.

Luc's review tag should be apply when it is accepted.The script does not add review tags by itself. I usually add it by hand.

If yo want to add the tags in the patch, you are welcome resend the patch with the new review tags. I just said don't need to send they if they are exactly the same.

In this case, it is likely to have new around of patches series. We can hold it off a bit.

Yes. That is right.

For binary op, most of the case it just need to do:
new flags = left flags | right flags;

Then check if the combined result has invalid result.

Yes, it would need to have "float constant" set on the final flag.

First of all, I think sparse will do implicit cast operation which will add the "float constant" flag. It is the case for non-constant operations. I need to dig deeper for constant case.

Even if there is no implicit cast operations, on the left hand you have "Int", on the right hand you have "Float". So the combination will have "Int", "Float", "composite op".

Base on my previous define for the arithmetic constant expressions:

The result has "Float" so it is not a integer constant expression.

The result does not have "Addr" so it is an arithmetic constant expression. Your example does not break my representation.

We can still adjust the final result as needed. For most of the case it does not need to adjust.

Other creative corner case are very welcome.

Chris

I appreciate your concerns but, please, don't bother much with this.

Luc

Hi Nicolai,

I finally finish up my more serious review of your patch series. So this email is going to be long. Sorry for the long delay.

Over all pretty good.

One thing has been puzzling me when I first read the patches, is about how the bits are used. Why there are some many bits need to be set when the expression is a very simple integer constant. Now I understand, the series basically treat each condition (is it a int constant, is it a constant expression) as a bit. So when you modify the int constant, the bits for other personality (int expression, arithmetic expression) has to be set as well. All this is for the optimization when you merge the binary operation when left and right expression.

I still see the multiple personality for constant expression (it can be both int constant and int expression etc) is a bit more complicate than it needs to, at least I now understand why you did it that way.

The detail review follows:

1) name of constexpr_flags is too long.

 struct expression {
        enum expression_type type:8;

- unsigned flags:8;
+ unsigned constexpr_flags:8;

I actually thing that the more generic name "flags" are fine without changing. If we need to add other flags not related to constant, we will need to rename it all over again.

If you still feel strongly about the flags being too generic. I suggest some thing shorter like "constant". The member is in an expression struct, so the "expr" is not needed.

2) CONSTEXPR_FLAG_XXX are being too long.

+       CONSTEXPR_FLAG_INT_CONST = (1 << 0),
+       CONSTEXPR_FLAG_FP_CONST = (1 << 1),
+       CONSTEXPR_FLAG_ENUM_CONST = (1 << 2),
+       CONSTEXPR_FLAG_CHAR_CONST = (1 << 3),
+
+       /*
+        * A constant expression in the sense of [6.6]:
+        * - integer constant expression [6.6(6)]
+        * - arithmetic constant expression [6.6(8)]
+        * - address constant [6.6(9)]
+        */
+       CONSTEXPR_FLAG_INT_CONST_EXPR = (1 << 4),
+       CONSTEXPR_FLAG_ARITH_CONST_EXPR = (1 << 5),
+       CONSTEXPR_FLAG_ADDR_CONST = (1 << 6),

This is really nick pick. Look at this defines, they all contain "CONSTEXPR_FLAG" and "_CONST", The only different is
a very short word "INT", "FP", "CHAR" etc. The signal ratio is pretty low.

I found it harder to read compare to the original enum value:

-enum {
- Int_const_expr = 1,
- Float_literal = 2,
-}; /* for expr->flags */

3) I think this is a typo, in the 01/21 patch
+/*
+ * Remove any "Constant" [6.4.4] flag, but retain the "constant
+ * expression" [6.6] flags.
+ */
+#define CONSTEXPR_FLAG_DECAY_CONSTS_MASK \
+ (CONSTEXPR_FLAG_INT_CONST | CONSTEXPR_FLAG_INT_CONST | \
+ CONSTEXPR_FLAG_FP_CONST | CONSTEXPR_FLAG_CHAR_CONST)

Did any one spot there are *two* duplicate CONSTEXPR_FLAG_INT_CONST there?

This is the point I try to make in 2), the very verbose long name actually hurt the readability. The eye need to move between long capital words try to extract the key difference. The key difference here is "INT" vs "FP" vs "CHAR".

I think if it was written as:

(Int_literal | Int_literal |
Float_literal | Char_literal)

The bug maybe would be easier to spot?

4) in patch 12:
static struct symbol *evaluate_ptr_add(struct expression *expr,
struct symbol *itype)

 {
        struct expression *index = expr->right;
        struct symbol *ctype, *base;
        int multiply;

        classify_type(degenerate(expr->left), &ctype);
        base = examine_pointer_target(ctype);

+ /*

+        * An address constant +/- an integer constant expression
+        * yields an address constant again [6.6(7)].
+        */

+ if ((expr->left->constexpr_flags & CONSTEXPR_FLAG_ADDR_CONST) &&
+ (expr->right->constexpr_flags & CONSTEXPR_FLAG_INT_CONST_EXPR))
+ expr->constexpr_flags = CONSTEXPR_FLAG_ADDR_CONST;
+
The constant flags should be set regardless. Otherwise it is not consistent with the rest of the code.

Also I found that code hard to read. Because the expression is really long, it break into a few lines.

I suggest move this code into a function:

           expr->const = evaluate_const_ptr_add(expr->right, expr->left);

Then inside evaluate_const_ptr_add() you can use early return to make the code does not need to use that long expression.

int evaluate_const_ptr_add(struct expression *left, struct expression *right)

{
          if (!(left->constant & Addr_const))
                   return 0;
          if (!(right->constant & Int_const_expr))
                   return 0;
          return Addr_const;
}

The same pattern can apply to a lot of the constant expression evaluation. A lot of them share the same function e.g. convert into constant int expression.

5) in Patch 1,4,8, a few patch combine into this result. static struct symbol *evaluate_logical(struct expression *expr)

 {
        if (!evaluate_conditional(expr->left, 0))
                return NULL;
        if (!evaluate_conditional(expr->right, 0))
                return NULL;

        /* the result is int [6.5.13(3), 6.5.14(3)] */
        expr->ctype = &int_ctype;

- if (expr->flags) {
- if (!(expr->left->flags & expr->right->flags & Int_const_expr))
- expr->flags = 0;
- }
+ expr->constexpr_flags = expr->left->constexpr_flags &

+               expr->right->constexpr_flags &
+               ~CONSTEXPR_FLAG_DECAY_CONSTS_MASK &
+               ~CONSTEXPR_FLAG_ADDR_CONST;
        return &int_ctype;

}

Again, with this complaxity and lone line, I suggest move into a function evaluate_const_int_expr(expr->left, expr->right)

Also this evaluate_const_int_expr() will be share by many similar caller. e.g. evaluate_compare etc.

6) In evaluate_conditional_expression cover by patch 3,8,11,18
+ /*

+        * A conditional operator yields a particular constant
+        * expression type only if all of its three subexpressions are
+        * of that type [6.6(6), 6.6(8)].
+        * As an extension, relax this restriction by allowing any
+        * constant expression type for the condition expression.
+        *
+        * A conditional operator never yields an address constant
+        * [6.6(9)].
+        * However, as an extension, if the condition is any constant
+        * expression, and the true and false expressions are both
+        * address constants, mark the result as an address constant.
+        */

+ if (expr->conditional->constexpr_flags &
+ (CONSTEXPR_FLAG_ARITH_CONST_EXPR | CONSTEXPR_FLAG_ADDR_CONST))
+ expr->constexpr_flags = (*true)->constexpr_flags &
+ expr->cond_false->constexpr_flags &
+ ~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;

This is another place hard to read. I also suggest always set the expr->constant flag. Also us3 a small function to wrap the complicate condition. The long expression really hurts here.

Chris

I'd suggest to also add on th same line a comment like:

	// used for the kinds of constant expression 

- Luc

...

Chris, I don't really understand your remark here (regardless what ?). Can you explain it a bit more please?

-- Luc

In has been a while. The currently code only touch the result constexpr_flags if left and right are pointer and integer. Otherwise, the constantexpr_flags are not
touch at all. It is not obvious what is the previous state of the constantexpr_flags.
I mean maybe it should clear it is that is not a constant? Most of the evaluate_xxx
function will just set the new constexpr_flags.

Chris

The very first patch of the series insure that all flags are initialized to the default 'NONE'. Then the next patches build on this and add info/bits to flags case by case.

As far as I can see the patch was correct.

-- Luc

How about a different test case. The right hand side is a const expression but not an int const expression.
So the result is not an address constant. The result should be some other kind of constant since both
left hand side and right hand side are both constant expression. If the result has NONE then it is wrong.

Chris

OK, I think I begin to understand.
It's important that the standard is not coherent on the notion of constant vs. constant expression.
This distinction exists for integers:
- a case in a switch statement requires an integer constant
- initializers are happy with integer constant expressions. For address, no such distinction exists, the standard specifically states in [6.6.9]:
An address constant is a null pointer, a pointer to an lvalue designating an object of static storage duration, or a pointer to a function designator; it shall be created explicitly using the unary & operator or an integer constant cast to pointer type, or implicitly by the use of an expression of array or function type. The array-subscript [] and member-access . and -> operators, the address & and indirection * unary operators, and pointer casts may be used in the creation of an address constant, but the value of an object shall not be accessed by use of these operators.

If the LHS is not an address constant, the result will not be any kind of constexprs. And if the LHS is an address constant but the RHS is not an integer constant expression then the result will also not be any kind of constexprs. Which leave us with the case covered here. Am I missing something?

-- Luc