[PATCH v3 00/21] improve constexpr handling

[PATCH v3 00/21] improve constexpr handling

[Nicolai Stange]

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

[PATCH v3 01/21] expression: introduce additional expression constness tracking flags

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 evaluate.c   | 83 ++++++++++++++++++++++++++++++++++++------------------------
 expand.c     | 10 ++++----
 expression.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------------
 expression.h | 65 +++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 171 insertions(+), 69 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 035e448..7bce4fb 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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;
diff --git a/expand.c b/expand.c
index 0f6720c..4739d54 100644
--- a/expand.c
+++ b/expand.c
@@ -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;
diff --git a/expression.c b/expression.c
index 7293d47..f9adab6 100644
--- a/expression.c
+++ b/expression.c
@@ -131,7 +131,8 @@ static struct token *parse_type(struct token *token, struct expression **tree)
 {
 	struct symbol *sym;
 	*tree = alloc_expression(token->pos, EXPR_TYPE);
-	(*tree)->flags = Int_const_expr; /* sic */
+	(*tree)->constexpr_flags =
+		CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK; /* sic */
 	token = typename(token, &sym, NULL);
 	if (sym->ident)
 		sparse_error(token->pos,
@@ -146,7 +147,7 @@ static struct token *builtin_types_compatible_p_expr(struct token *token,
 {
 	struct expression *expr = alloc_expression(
 		token->pos, EXPR_COMPARE);
-	expr->flags = Int_const_expr;
+	expr->constexpr_flags = CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 	expr->op = SPECIAL_EQUAL;
 	token = token->next;
 	if (!match_op(token, '('))
@@ -200,7 +201,8 @@ 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->flags = Int_const_expr;
+			e->constexpr_flags =
+				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			e->op = '[';
 			*p = e;
 			p = &e->down;
@@ -208,7 +210,8 @@ static struct token *builtin_offsetof_expr(struct token *token,
 		case '.':
 			token = token->next;
 			e = alloc_expression(token->pos, EXPR_OFFSETOF);
-			e->flags = Int_const_expr;
+			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");
@@ -220,7 +223,8 @@ static struct token *builtin_offsetof_expr(struct token *token,
 		case '[':
 			token = token->next;
 			e = alloc_expression(token->pos, EXPR_OFFSETOF);
-			e->flags = Int_const_expr;
+			e->constexpr_flags =
+				CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 			e->op = '[';
 			token = parse_expression(token, &e->index);
 			token = expect(token, ']',
@@ -336,7 +340,7 @@ got_it:
 			"likely to produce unsigned long (and a warning) here",
 			show_token(token));
         expr->type = EXPR_VALUE;
-	expr->flags = Int_const_expr;
+	expr->constexpr_flags = CONSTEXPR_FLAG_INT_CONST_SET_MASK;
         expr->ctype = ctype_integer(size, want_unsigned);
         expr->value = value;
 	return;
@@ -361,7 +365,7 @@ Float:
 	else
 		goto Enoint;
 
-	expr->flags = Float_literal;
+	expr->constexpr_flags = CONSTEXPR_FLAG_FP_CONST_SET_MASK;
 	expr->type = EXPR_FVALUE;
 	return;
 
@@ -375,8 +379,8 @@ struct token *primary_expression(struct token *token, struct expression **tree)
 
 	switch (token_type(token)) {
 	case TOKEN_CHAR ... TOKEN_WIDE_CHAR_EMBEDDED_3:
-		expr = alloc_expression(token->pos, EXPR_VALUE);   
-		expr->flags = Int_const_expr;
+		expr = alloc_expression(token->pos, EXPR_VALUE);
+		expr->constexpr_flags = CONSTEXPR_FLAG_CHAR_CONST_SET_MASK;
 		expr->ctype = token_type(token) < TOKEN_WIDE_CHAR ? &int_ctype : &long_ctype;
 		get_char_constant(token, &expr->value);
 		token = token->next;
@@ -390,7 +394,7 @@ struct token *primary_expression(struct token *token, struct expression **tree)
 
 	case TOKEN_ZERO_IDENT: {
 		expr = alloc_expression(token->pos, EXPR_SYMBOL);
-		expr->flags = Int_const_expr;
+		expr->constexpr_flags = CONSTEXPR_FLAG_INT_CONST_SET_MASK;
 		expr->ctype = &int_ctype;
 		expr->symbol = &zero_int;
 		expr->symbol_name = token->ident;
@@ -417,7 +421,8 @@ struct token *primary_expression(struct token *token, struct expression **tree)
 			*expr = *sym->initializer;
 			/* we want the right position reported, thus the copy */
 			expr->pos = token->pos;
-			expr->flags = Int_const_expr;
+			expr->constexpr_flags =
+				CONSTEXPR_FLAG_ENUM_CONST_SET_MASK;
 			token = next;
 			break;
 		}
@@ -452,12 +457,15 @@ struct token *primary_expression(struct token *token, struct expression **tree)
 			expr->op = '(';
 			token = parens_expression(token, &expr->unop, "in expression");
 			if (expr->unop)
-				expr->flags = expr->unop->flags;
+				expr->constexpr_flags =
+					expr->unop->constexpr_flags;
 			break;
 		}
 		if (token->special == '[' && lookup_type(token->next)) {
 			expr = alloc_expression(token->pos, EXPR_TYPE);
-			expr->flags = Int_const_expr; /* sic */
+			/* 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;
@@ -573,7 +581,8 @@ static struct token *type_info_expression(struct token *token,
 	struct token *p;
 
 	*tree = expr;
-	expr->flags = Int_const_expr; /* XXX: VLA support will need that changed */
+	/* XXX: VLA support will need that changed */
+	expr->constexpr_flags = CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK;
 	token = token->next;
 	if (!match_op(token, '(') || !lookup_type(token->next))
 		return unary_expression(token, &expr->cast_expression);
@@ -662,7 +671,8 @@ 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->flags = unop->flags & Int_const_expr;
+			unary->constexpr_flags = unop->constexpr_flags &
+				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 			*tree = unary;
 			return next;
 		}
@@ -720,10 +730,27 @@ static struct token *cast_expression(struct token *token, struct expression **tr
 			if (!v)
 				return token;
 			cast->cast_expression = v;
-			if (v->flags & Int_const_expr)
-				cast->flags = Int_const_expr;
-			else if (v->flags & Float_literal) /* and _not_ int */
-				cast->flags = Int_const_expr | Float_literal;
+
+			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;
 		}
 	}
@@ -760,8 +787,10 @@ 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->flags = left->flags & right->flags		\
-						& Int_const_expr;	\
+			top->constexpr_flags =				\
+				left->constexpr_flags &		\
+				right->constexpr_flags &		\
+				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;	\
 			top->op = op;					\
 			top->left = left;				\
 			top->right = right;				\
@@ -865,12 +894,13 @@ struct token *conditional_expression(struct token *token, struct expression **tr
 		token = expect(token, ':', "in conditional expression");
 		token = conditional_expression(token, &expr->cond_false);
 		if (expr->left && expr->cond_false) {
-			int is_const = expr->left->flags &
-					expr->cond_false->flags &
-					Int_const_expr;
+			expr->constexpr_flags = expr->left->constexpr_flags &
+				expr->cond_false->constexpr_flags;
 			if (expr->cond_true)
-				is_const &= expr->cond_true->flags;
-			expr->flags = is_const;
+				expr->constexpr_flags &=
+					expr->cond_true->constexpr_flags;
+			expr->constexpr_flags &=
+				~CONSTEXPR_FLAG_DECAY_CONSTS_MASK;
 		}
 	}
 	return token;
diff --git a/expression.h b/expression.h
index 80b3be5..3725a73 100644
--- a/expression.h
+++ b/expression.h
@@ -66,10 +66,65 @@ enum expression_type {
 	EXPR_OFFSETOF,
 };
 
-enum {
-	Int_const_expr = 1,
-	Float_literal = 2,
-}; /* for expr->flags */
+
+/*
+ * Flags for tracking the promotion of constness related attributes
+ * from subexpressions to their parents.
+ *
+ * The flags are not independent as one might imply another.
+ * The implications are as follows:
+ * - CONSTEXPR_FLAG_INT_CONST, CONSTEXPR_FLAG_ENUM_CONST and
+ *   CONSTEXPR_FLAG_CHAR_CONST imply CONSTEXPR_FLAG_INT_CONST_EXPR.
+ *
+ * Use the CONSTEXPR_FLAG_*_SET_MASK and CONSTEXPR_FLAG_*_CLEAR_MASK
+ * helper macros defined below to set or clear one of these flags.
+ */
+enum constexpr_flag {
+	CONSTEXPR_FLAG_NONE = 0,
+	/*
+	 * A constant in the sense of [6.4.4]:
+	 * - Integer constant [6.4.4.1]
+	 * - Floating point constant [6.4.4.2]
+	 * - Enumeration constant [6.4.4.3]
+	 * - Character constant [6.4.4.4]
+	 */
+	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)]
+	 */
+	CONSTEXPR_FLAG_INT_CONST_EXPR = (1 << 4),
+};
+
+#define CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK	\
+	(CONSTEXPR_FLAG_INT_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)
+
+/* enumeration constant => integer constant expression */
+#define CONSTEXPR_FLAG_ENUM_CONST_SET_MASK				\
+	(CONSTEXPR_FLAG_ENUM_CONST | CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK)
+
+/* character constant => integer constant expression */
+#define CONSTEXPR_FLAG_CHAR_CONST_SET_MASK			\
+	(CONSTEXPR_FLAG_CHAR_CONST | CONSTEXPR_FLAG_INT_CONST_EXPR_SET_MASK)
+
+/*
+ * 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)
 
 enum {
 	Taint_comma = 1,
@@ -77,7 +132,7 @@ enum {
 
 struct expression {
 	enum expression_type type:8;
-	unsigned flags:8;
+	unsigned constexpr_flags:8;
 	int op;
 	struct position pos;
 	struct symbol *ctype;
-- 
2.7.0

[PATCH v3 02/21] expression: init constexpr_flags at expression allocation

[Nicolai Stange]

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().

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 expression.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/expression.h b/expression.h
index 3725a73..8a943a4 100644
--- a/expression.h
+++ b/expression.h
@@ -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

[PATCH v3 03/21] expression: examine constness of casts at evaluation only

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index 7bce4fb..24a7f2f 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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
diff --git a/expression.c b/expression.c
index f9adab6..759bee8 100644
--- a/expression.c
+++ b/expression.c
@@ -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;
 		}
 	}
diff --git a/expression.h b/expression.h
index 8a943a4..408382e 100644
--- a/expression.h
+++ b/expression.h
@@ -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.
  */
diff --git a/validation/constexpr-cast.c b/validation/constexpr-cast.c
new file mode 100644
index 0000000..2706961
--- /dev/null
+++ b/validation/constexpr-cast.c
@@ -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

[PATCH v3 04/21] expression: examine constness of binops and alike at evaluation only

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index 24a7f2f..32e5930 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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))
diff --git a/expression.c b/expression.c
index 759bee8..5970b1b 100644
--- a/expression.c
+++ b/expression.c
@@ -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;				\
diff --git a/validation/constexpr-binop.c b/validation/constexpr-binop.c
new file mode 100644
index 0000000..85a88e3
--- /dev/null
+++ b/validation/constexpr-binop.c
@@ -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

[PATCH v3 05/21] expression: examine constness of preops at evaluation only

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index 32e5930..43a0432 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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)) {
diff --git a/expression.c b/expression.c
index 5970b1b..302ef8b 100644
--- a/expression.c
+++ b/expression.c
@@ -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;
 		}
diff --git a/validation/constexpr-preop.c b/validation/constexpr-preop.c
new file mode 100644
index 0000000..5d869da
--- /dev/null
+++ b/validation/constexpr-preop.c
@@ -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

[PATCH v3 06/21] expression: examine constness of conditionals at evaluation only

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index 43a0432..03db2c3 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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);
diff --git a/expression.c b/expression.c
index 302ef8b..b2d5eb4 100644
--- a/expression.c
+++ b/expression.c
@@ -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;
 }
diff --git a/validation/constexpr-conditional.c b/validation/constexpr-conditional.c
new file mode 100644
index 0000000..a3331b3
--- /dev/null
+++ b/validation/constexpr-conditional.c
@@ -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

[PATCH v3 07/21] expression: add support for tagging arithmetic constant expressions

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 expression.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/expression.h b/expression.h
index 408382e..b85f1b1 100644
--- a/expression.h
+++ b/expression.h
@@ -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

[PATCH v3 08/21] expression, evaluate: add support for tagging address constants

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 evaluate.c   | 27 ++++++++++++++++++++++-----
 expression.h |  2 ++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 03db2c3..dd44cd5 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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)].
diff --git a/expression.h b/expression.h
index b85f1b1..285c18c 100644
--- a/expression.h
+++ b/expression.h
@@ -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

[PATCH v3 09/21] evaluate: check static storage duration objects' intializers' constness

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index dd44cd5..300bfbe 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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;
 	}
 
diff --git a/lib.c b/lib.c
index 8dc5bcf..75cea42 100644
--- a/lib.c
+++ b/lib.c
@@ -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 },
diff --git a/lib.h b/lib.h
index 15b69fa..916eb31 100644
--- a/lib.h
+++ b/lib.h
@@ -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;
diff --git a/sparse.1 b/sparse.1
index 4adaf6c..7117bdf 100644
--- a/sparse.1
+++ b/sparse.1
@@ -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.
 
diff --git a/validation/constexpr-init.c b/validation/constexpr-init.c
new file mode 100644
index 0000000..d7e7a45
--- /dev/null
+++ b/validation/constexpr-init.c
@@ -0,0 +1,60 @@
+static int a = 1;					// OK
+static int b[2] = {1, 1};				// OK
+static void c(void) {}
+
+struct A {
+	int a;
+	int b[2];
+};
+
+struct B {
+	int c;
+	struct A d;
+};
+
+static struct B d= {1, {1, {1, 1}}};				// OK
+static struct B e= {a, {1, {1, 1}}};				// KO
+static struct B f= {1, {a, {1, 1}}};				// KO
+static struct B g= {1, {1, {a, 1}}};				// KO
+static struct B h= {1, {1, {1, a}}};				// KO
+static struct B i= {.c = 1, .d = {.a = 1, .b = {1, 1}}};	// OK
+static struct B j= {.c = a, .d = {.a = 1, .b = {1, 1}}};	// KO
+static struct B k= {.c = 1, .d = {.a = a, .b = {1, 1}}};	// KO
+static struct B l= {.c = 1, .d = {.a = 1, .b = {a, 1}}};	// KO
+static struct B m= {.c = 1, .d = {.a = 1, .b = {1, a}}};	// KO
+
+static int n[] = {a, 1};				// KO
+static int o[] = {1, a};				// KO
+static int p[] = {[0] = a, [1] = 1};			// KO
+static int q[] = {[0] = 1, [1] = a};			// KO
+
+static void r(void) {
+	int a = 0;
+	int b = a;		// OK
+}
+
+static void s(void) {
+	int a = 1;
+	static int b = a;	// KO
+}
+
+/*
+ * check-name: static storage object initializer constness verification.
+ * check-command: sparse -Wconstexpr-not-const $file
+ *
+ * check-error-start
+constexpr-init.c:16:21: warning: non-constant initializer for static object
+constexpr-init.c:17:25: warning: non-constant initializer for static object
+constexpr-init.c:18:29: warning: non-constant initializer for static object
+constexpr-init.c:19:32: warning: non-constant initializer for static object
+constexpr-init.c:21:26: warning: non-constant initializer for static object
+constexpr-init.c:22:40: warning: non-constant initializer for static object
+constexpr-init.c:23:49: warning: non-constant initializer for static object
+constexpr-init.c:24:52: warning: non-constant initializer for static object
+constexpr-init.c:26:19: warning: non-constant initializer for static object
+constexpr-init.c:27:22: warning: non-constant initializer for static object
+constexpr-init.c:28:25: warning: non-constant initializer for static object
+constexpr-init.c:29:34: warning: non-constant initializer for static object
+constexpr-init.c:38:24: warning: non-constant initializer for static object
+ * check-error-end
+ */
-- 
2.7.0

[PATCH v3 10/21] expression, evaluate: recognize static objects as address constants

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index 300bfbe..91f89f4 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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;
diff --git a/expression.c b/expression.c
index b2d5eb4..11fb9cd 100644
--- a/expression.c
+++ b/expression.c
@@ -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;
 	}
diff --git a/validation/constexpr-addr-of-static.c b/validation/constexpr-addr-of-static.c
new file mode 100644
index 0000000..a3af99a
--- /dev/null
+++ b/validation/constexpr-addr-of-static.c
@@ -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

[PATCH v3 11/21] evaluate: recognize address constants created through casts

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 evaluate.c                          | 10 +++++++++-
 validation/constexpr-pointer-cast.c | 13 +++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 validation/constexpr-pointer-cast.c

diff --git a/evaluate.c b/evaluate.c
index 91f89f4..a740474 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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)].
diff --git a/validation/constexpr-pointer-cast.c b/validation/constexpr-pointer-cast.c
new file mode 100644
index 0000000..d19c108
--- /dev/null
+++ b/validation/constexpr-pointer-cast.c
@@ -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

[PATCH v3 12/21] evaluate: recognize address constants created through pointer arithmetic

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 evaluate.c                           |  8 ++++++++
 validation/constexpr-pointer-arith.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 validation/constexpr-pointer-arith.c

diff --git a/evaluate.c b/evaluate.c
index a740474..16b4ce0 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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;
diff --git a/validation/constexpr-pointer-arith.c b/validation/constexpr-pointer-arith.c
new file mode 100644
index 0000000..a922028
--- /dev/null
+++ b/validation/constexpr-pointer-arith.c
@@ -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

[PATCH v3 13/21] evaluate: recognize members of static compound objects as address constants

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index 16b4ce0..f31ba9c 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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;
 }
 
diff --git a/validation/constexpr-addr-of-static-member.c b/validation/constexpr-addr-of-static-member.c
new file mode 100644
index 0000000..f944f21
--- /dev/null
+++ b/validation/constexpr-addr-of-static-member.c
@@ -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

[PATCH v3 14/21] evaluate: recognize string literals as address constants

[Nicolai Stange]

Introduce support for recognizing string literals as address constants.

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

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 evaluate.c                    | 1 +
 validation/constexpr-string.c | 9 +++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 validation/constexpr-string.c

diff --git a/evaluate.c b/evaluate.c
index f31ba9c..7a4af39 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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 = '*';
diff --git a/validation/constexpr-string.c b/validation/constexpr-string.c
new file mode 100644
index 0000000..e641a83
--- /dev/null
+++ b/validation/constexpr-string.c
@@ -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

[PATCH v3 15/21] expression: recognize references to labels as address constants

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 expression.c                    |  1 +
 validation/constexpr-labelref.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+)
 create mode 100644 validation/constexpr-labelref.c

diff --git a/expression.c b/expression.c
index 11fb9cd..ac6cf43 100644
--- a/expression.c
+++ b/expression.c
@@ -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;
diff --git a/validation/constexpr-labelref.c b/validation/constexpr-labelref.c
new file mode 100644
index 0000000..c51f9c8
--- /dev/null
+++ b/validation/constexpr-labelref.c
@@ -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

[PATCH v3 16/21] expression: examine constness of __builtin_offsetof at evaluation only

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index 7a4af39..0101e61 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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) {
diff --git a/expression.c b/expression.c
index ac6cf43..c3d54fe 100644
--- a/expression.c
+++ b/expression.c
@@ -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, ']',
diff --git a/validation/constexpr-offsetof.c b/validation/constexpr-offsetof.c
new file mode 100644
index 0000000..d1697b0
--- /dev/null
+++ b/validation/constexpr-offsetof.c
@@ -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

[PATCH v3 17/21] symbol: flag builtins constant_p, safe_p and warning as constexprs

[Nicolai Stange]

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

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 symbol.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/symbol.c b/symbol.c
index 0ceff62..42e6a8f 100644
--- a/symbol.c
+++ b/symbol.c
@@ -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

[PATCH v3 18/21] evaluate: relax some constant expression rules for pointer expressions

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 evaluate.c | 38 ++++++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 0101e61..ff51d84 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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

[PATCH v3 19/21] expression, evaluate: support compound literals as address constants

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index ff51d84..06c360f 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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 = '*';
diff --git a/expression.c b/expression.c
index c3d54fe..f49c8f6 100644
--- a/expression.c
+++ b/expression.c
@@ -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");
diff --git a/validation/constexpr-compound-literal.c b/validation/constexpr-compound-literal.c
new file mode 100644
index 0000000..d7f21ad
--- /dev/null
+++ b/validation/constexpr-compound-literal.c
@@ -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

[PATCH v3 20/21] symbol: do not inherit storage modifiers from base types at examination

[Nicolai Stange]

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().

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 symbol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/symbol.c b/symbol.c
index 42e6a8f..4766adb 100644
--- a/symbol.c
+++ b/symbol.c
@@ -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

[PATCH v3 21/21] evaluation: treat comparsions between types as integer constexpr

[Nicolai Stange]

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.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 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

diff --git a/evaluate.c b/evaluate.c
index 06c360f..31ac4e1 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -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,
diff --git a/expression.c b/expression.c
index f49c8f6..4285cc8 100644
--- a/expression.c
+++ b/expression.c
@@ -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;
diff --git a/validation/constexpr-types-compatible-p.c b/validation/constexpr-types-compatible-p.c
new file mode 100644
index 0000000..78c37b4
--- /dev/null
+++ b/validation/constexpr-types-compatible-p.c
@@ -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

Re: [PATCH v3 00/21] improve constexpr handling

[Nicolai Stange]

Nicolai Stange <nicstange@gmail.com> writes:

> Here comes the greatly enhanced v3 of this series.

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


>
> 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

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

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



On Fri, Feb 19, 2016 at 4:22 PM, Nicolai Stange <nicstange@gmail.com> wrote:
> Nicolai Stange <nicstange@gmail.com> writes:
>
>> Here comes the greatly enhanced v3 of this series.
>
> Just a gently push to get some reviews on this...
>

Re: [PATCH v3 00/21] improve constexpr handling

[Nicolai Stange]

Hi Chris,

Christopher Li <sparse@chrisli.org> writes:

> Sorry for the late reply.

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

> 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.

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

>
> +
> +       /*
> +        * 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.

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)].

>
> 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.
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.

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


Thank you for reviewing!

Nicolai

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Fri, Feb 19, 2016 at 09:22:50AM +0100, Nicolai Stange wrote:
> Nicolai Stange <nicstange@gmail.com> writes:
> 
> > Here comes the greatly enhanced v3 of this series.
> 
> Just a gently push to get some reviews on this...
> 

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


Regards,
Luc Van Oostenryck

Re: [PATCH v3 02/21] expression: init constexpr_flags at expression allocation

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:30:49AM +0100, Nicolai Stange wrote:
> 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().
> 
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> ---
>  expression.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/expression.h b/expression.h
> index 3725a73..8a943a4 100644
> --- a/expression.h
> +++ b/expression.h
> @@ -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;
>  }

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 04/21] expression: examine constness of binops and alike at evaluation only

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:32:32AM +0100, Nicolai Stange wrote:
> 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.
> 

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
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 05/21] expression: examine constness of preops at evaluation only

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:33:38AM +0100, Nicolai Stange wrote:
> 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.


Fien for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 06/21] expression: examine constness of conditionals at evaluation only

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:34:34AM +0100, Nicolai Stange wrote:
> 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.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 07/21] expression: add support for tagging arithmetic constant expressions

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:35:27AM +0100, Nicolai Stange wrote:
> 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.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 08/21] expression, evaluate: add support for tagging address constants

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:36:18AM +0100, Nicolai Stange wrote:
> 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.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 09/21] evaluate: check static storage duration objects' intializers' constness

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:37:18AM +0100, Nicolai Stange wrote:
> 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.


"-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.
 
> 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.
> 
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> ---
> diff --git a/evaluate.c b/evaluate.c
> index dd44cd5..300bfbe 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -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;

This saving/restoring of the modifiers is not very nice.
Hadn't we talked about adding an arg to handle_simple_initializer() or so?
  
>  		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)

This last line whould be more indented, maybe like the previous line.

> +			warning(e->pos, "non-constant initializer for static object");
> +
>  		return 1;
>  	}
>  
> diff --git a/lib.c b/lib.c
> index 8dc5bcf..75cea42 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -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;


This name is quite good.

Re: [PATCH v3 10/21] expression, evaluate: recognize static objects as address constants

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:38:24AM +0100, Nicolai Stange wrote:
> 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.
> 
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> ---
> diff --git a/evaluate.c b/evaluate.c
> index 300bfbe..91f89f4 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -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;

I'm not sure to follow this one.
What are the possible value of expr->constexpr_flags at this point?


> diff --git a/expression.c b/expression.c
> index b2d5eb4..11fb9cd 100644
> --- a/expression.c
> +++ b/expression.c
> @@ -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;


Missing space after the 'if'

Re: [PATCH v3 11/21] evaluate: recognize address constants created through casts

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:39:09AM +0100, Nicolai Stange wrote:
> 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.
> 
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> ---
>  evaluate.c                          | 10 +++++++++-
>  validation/constexpr-pointer-cast.c | 13 +++++++++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
>  create mode 100644 validation/constexpr-pointer-cast.c
> 
> diff --git a/evaluate.c b/evaluate.c
> index 91f89f4..a740474 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -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 &&

Better put "class1 & TYPE_PTR" between parenthesis.

> +			(target->constexpr_flags & CONSTEXPR_FLAG_INT_CONST))
> +			expr->constexpr_flags = CONSTEXPR_FLAG_ADDR_CONST;
> +	} else {

I would have put as the else clause instead of changing the condition.
Not that it really matters though.

Re: [PATCH v3 12/21] evaluate: recognize address constants created through pointer arithmetic

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:39:58AM +0100, Nicolai Stange wrote:
> 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.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 13/21] evaluate: recognize members of static compound objects as address constants

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:40:42AM +0100, Nicolai Stange wrote:
> 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.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 14/21] evaluate: recognize string literals as address constants

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:41:24AM +0100, Nicolai Stange wrote:
> Introduce support for recognizing string literals as address constants.
> 
> Make evaluate_string() unconditionally tag the *-preop wrapped symbol
> expression as being an address constant.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 15/21] expression: recognize references to labels as address constants

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:42:10AM +0100, Nicolai Stange wrote:
> 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.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 18/21] evaluate: relax some constant expression rules for pointer expressions

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:44:38AM +0100, Nicolai Stange wrote:
> 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.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 18/21] evaluate: relax some constant expression rules for pointer expressions

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:44:38AM +0100, Nicolai Stange wrote:
> 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.
> 
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> ---
> diff --git a/evaluate.c b/evaluate.c
> index 0101e61..ff51d84 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -1129,14 +1129,23 @@ static struct symbol *evaluate_conditional_expression(struct expression *expr)
 ...
>  	 * 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.
>  	 */

The comment make perfect sense in the patch serie because it explain exactly
what the incremental patch is doing.
But once the patch is applied it's not what the code is really doing:
	the result is not marked as an address constant,
	it's only not unmarked anymore.
So, I think it's better to restrict it to something like:
	However as an extension also accept 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))

The 'if' expression could be simplified (!CONSTEXPR_FLAG_NON)

> +		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);

Re: [PATCH v3 18/21] evaluate: relax some constant expression rules for pointer expressions

[Luc Van Oostenryck]

On Tue, Mar 15, 2016 at 06:47:49PM +0100, Luc Van Oostenryck wrote:
> On Mon, Feb 01, 2016 at 03:44:38AM +0100, Nicolai Stange wrote:
> > 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.
> > 
> 
> Fine for me.
> 
> Feel free to add
>   Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>


Sorry that wasn't for this patch

Re: [PATCH v3 17/21] symbol: flag builtins constant_p, safe_p and warning as constexprs

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:43:53AM +0100, Nicolai Stange wrote:
> Unconditionally flag the expressions
>   __builtin_constant_p(),
>   __builtin_safe_p(),
>   __builtin_warning()
> as being integer constant expressions.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 16/21] expression: examine constness of __builtin_offsetof at evaluation only

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:42:59AM +0100, Nicolai Stange wrote:
> 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.

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.
 
> Furthermore, carry an array index expression's constness flags
> through the implicit cast to size_t type.
> 
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> ---
> diff --git a/evaluate.c b/evaluate.c
> index 7a4af39..0101e61 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -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;

This saving/restoring of teh flags is annoying.
Why is it needed? Can it be solved in another way?

Re: [PATCH v3 19/21] expression, evaluate: support compound literals as address constants

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:45:27AM +0100, Nicolai Stange wrote:
> 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.
> 

...

> diff --git a/validation/constexpr-compound-literal.c b/validation/constexpr-compound-literal.c
> --- /dev/null
> +++ b/validation/constexpr-compound-literal.c
> @@ -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

Humm ... why is this last one different than the corresponding 
top level one @ line 1?

Re: [PATCH v3 20/21] symbol: do not inherit storage modifiers from base types at examination

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:46:31AM +0100, Nicolai Stange wrote:
> 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().
> 
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> ---
>  symbol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/symbol.c b/symbol.c
> index 42e6a8f..4766adb 100644
> --- a/symbol.c
> +++ b/symbol.c
> @@ -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) {
> -- 


Mmmm ...

As I already said in the v2 of this serie, it's a complicated case,
and yes this change certainly quite the warning.
However I don't think it's the right fix.

For info, my previous analysis & comment was:
	It should be noted that the warning only occurs because:
	1) the function is inlined.
	2) the function is variadic and the symbol is given in the variable part.
	3) the symbol doesn't need integer promotion.
	4) MOD_STORAGE is part of MOD_PTRINHERIT.
	
	It would be good to show these conditions in the test case.
	
	What I see in the code about calls is that when conditions 1, 2 & 3 are met
	the argument to this variadic inline function is sorta used as-it-is while,
	I think, a sort of l-value to r-value conversion should be done.
	This conversion would essentially just adapting some of the modifiers:
	clearing MOD_STORAGE (and maybe MOD_ADDRESSABLE).
	
	For the condition 4), while digging in the code history (commit 822d3b1a222b),
	it can be seen that MOD_STORAGE was added to MOD_PTRINHERIT without a real need;
	quoting Linus:
		This still keeps other attributes, like "pointer to 'static'". Useful?
		Probably not. I'll have to think about it.
	
	Normally it's not a problem to have it because unneeded modifiers are generally
	filtered out or simply ignored.
	
	So there is several ways to "fix" (or at least hide) this problem but it's not
	clear to me what really should be done.

However, some time ago I sent another patch for 2)
Chris has already taken it:
	7647c775497f ("Do not drop 'nocast' modifier when taking the address")
even thought it doesn't yet appears on sparse/sparse.git

So, once this last one will be on the master tree, this patch will be moot.


Luc

Re: [PATCH v3 21/21] evaluation: treat comparsions between types as integer constexpr

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:47:16AM +0100, Nicolai Stange wrote:
> 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.

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 03/21] expression: examine constness of casts at evaluation only

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:31:47AM +0100, Nicolai Stange wrote:
> 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.
> 

Fine for me.

Feel free to add
  Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH v3 01/21] expression: introduce additional expression constness tracking flags

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:29:40AM +0100, Nicolai Stange wrote:
> 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.
> 

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

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Mon, Feb 01, 2016 at 03:28:38AM +0100, Nicolai Stange wrote:
> Here comes the greatly enhanced v3 of this series.
> 


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

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Tue, Mar 15, 2016 at 11:36:17PM +0100, Luc Van Oostenryck wrote:
> On Mon, Feb 01, 2016 at 03:28:38AM +0100, Nicolai Stange wrote:
> > Here comes the greatly enhanced v3 of this series.
> > 
> 
> 
> 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?
> 
Chris, is it possible to have some feedback on this, please?

Is there anything more I can do to help?


Best regards,

Luc

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Mon, Feb 1, 2016 at 10:28 AM, Nicolai Stange <nicstange@gmail.com> wrote:
> Here comes the greatly enhanced v3 of this series.
>
> Luc's suggestions about splitting some patches turned out to be very fruitful!

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

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Wed, Nov 23, 2016 at 11:12:02AM +0800, Christopher Li wrote:
> +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,"

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.

> +       [(int)(float)0.] = 0,   // KO
> +
> +       [(int)(void*)0] = 0,    // KO
> Also this one should be fine?

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

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Wed, Nov 23, 2016 at 12:05 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> 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.

I see, thanks for pointing it out.

>> +       [(int)(float)0.] = 0,   // KO
>> +
>> +       [(int)(void*)0] = 0,    // KO
>> Also this one should be fine?
>
> same for '(void *) 0'.
>

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

Re: [PATCH v3 00/21] improve constexpr handling

[Nicolai Stange]

Hi Christopher,

Christopher Li <sparse@chrisli.org> writes:

> On Wed, Nov 23, 2016 at 12:05 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> 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.
>
> I see, thanks for pointing it out.
>
>>> +       [(int)(float)0.] = 0,   // KO
>>> +
>>> +       [(int)(void*)0] = 0,    // KO
>>> Also this one should be fine?
>>
>> same for '(void *) 0'.
>>
>
> 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?

(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.


> 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.

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


> 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.

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.


> 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.

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

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Wed, Nov 23, 2016 at 4:39 PM, Nicolai Stange <nicstange@gmail.com> wrote:
> (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).

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?

>> 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.
>
> Sorry, I don't understand that last sentence. "Set" in the mathematical
> sense? How would that look like?

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.

>
>
>> 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.
>
> 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.

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

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

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

Re: [PATCH v3 00/21] improve constexpr handling

[Nicolai Stange]

Christopher Li <sparse@chrisli.org> writes:

> On Wed, Nov 23, 2016 at 4:39 PM, Nicolai Stange <nicstange@gmail.com> wrote:
>> (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).
>
> 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?

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

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

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.


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

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.


>
>>> 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.
>>
>> Sorry, I don't understand that last sentence. "Set" in the mathematical
>> sense? How would that look like?
>
> 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.

+ 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).


>
> 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.
>
>>
>>
>>> 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.
>>
>> 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.
>
> Good, another corner case condition. That meas the arithmetic bits is here
> to stay. BTW, your test case is very good.

Thanks :)


Nicolai

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Thu, Nov 24, 2016 at 12:43 AM, Nicolai Stange <nicstange@gmail.com> wrote:
>
> I don't think that this would qualify as a constant expression of any
> type.
>
>> How about (void*)(0 + 'a') and (void*)0 + 'a', are they address constant
>> or not?
>
> 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.

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.

>>
>> Basically, there are six bits as basic element. One bit per element.
>> Integer constant, enum constant, char constant, float constant,
>> address constant,
>> arithmetic constant.
>
> + 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).

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

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Thu, Nov 24, 2016 at 1:38 AM, Christopher Li <sparse@chrisli.org> wrote:

> 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.

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

Chris

Re: [PATCH v3 00/21] improve constexpr handling

[Nicolai Stange]

Christopher Li <sparse@chrisli.org> writes:

> On Thu, Nov 24, 2016 at 12:43 AM, Nicolai Stange <nicstange@gmail.com> wrote:
>>
>> I don't think that this would qualify as a constant expression of any
>> type.
>>
>>> How about (void*)(0 + 'a') and (void*)0 + 'a', are they address constant
>>> or not?
>>
>> 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.
>
> 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.

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).


> In your patch:

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

> 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.

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.


>>>
>>> Basically, there are six bits as basic element. One bit per element.
>>> Integer constant, enum constant, char constant, float constant,
>>> address constant,
>>> arithmetic constant.
>>
>> + 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).
>
> 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?

(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

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Thu, Nov 24, 2016 at 2:33 AM, Nicolai Stange <nicstange@gmail.com> wrote:
>
> 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).

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

>
> Can you tell which one? If not, would resending this series help?
>
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.


> 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.

Never mind that.

>> 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?
>
> (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.
>

(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

Re: [PATCH v3 00/21] improve constexpr handling

[Nicolai Stange]

Christopher Li <sparse@chrisli.org> writes:

> On Thu, Nov 24, 2016 at 2:33 AM, Nicolai Stange <nicstange@gmail.com> wrote:
>>
>> 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).
>
> Good. Thanks for reminding of that. So ideally this is under a more
> strict options.

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.


> 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.

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.


>>> 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?
>>
>> (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.
>>
>
> (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.

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


> 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.

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

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Thu, Nov 24, 2016 at 5:45 PM, Nicolai Stange <nicstange@gmail.com> wrote:
> 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.

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

> 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.

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, 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.
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.

> 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?

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:

>>> Integer constant expression can be tested as:
>>>
>>> !(flags & ( Addr | Float) ) && flag

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

>>>
>>> Arithmetic constant expression can be tested as:
>>>
>>> !(flags & Addr) ) && flag
>>>

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

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Thu, Nov 24, 2016 at 07:24:51PM +0800, Christopher Li wrote:
> On Thu, Nov 24, 2016 at 5:45 PM, Nicolai Stange <nicstange@gmail.com> wrote:
> > 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.
> 
> 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.

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

Luc

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

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

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Tue, Dec 06, 2016 at 02:00:50PM +0800, Christopher Li wrote:
> 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.

I'd suggest to also add on th same line a comment like:
	// used for the kinds of constant expression 

- Luc

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Tue, Dec 06, 2016 at 02:00:50PM +0800, Christopher Li wrote:
> 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.
> 
...
> 
> 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.

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

-- Luc

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Wed, Mar 29, 2017 at 10:42 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Tue, Dec 06, 2016 at 02:00:50PM +0800, Christopher Li wrote:
>> The constant flags should be set regardless. Otherwise it is not consistent
>> with the rest of the code.
>
> Chris, I don't really understand your remark here (regardless what ?).
> Can you explain it a bit more please?

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

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Fri, Mar 31, 2017 at 7:06 AM, Christopher Li <sparse@chrisli.org> wrote:
> On Wed, Mar 29, 2017 at 10:42 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Tue, Dec 06, 2016 at 02:00:50PM +0800, Christopher Li wrote:
>>> The constant flags should be set regardless. Otherwise it is not consistent
>>> with the rest of the code.
>>
>> Chris, I don't really understand your remark here (regardless what ?).
>> Can you explain it a bit more please?
>
> 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.

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

Re: [PATCH v3 00/21] improve constexpr handling

[Christopher Li]

On Fri, Mar 31, 2017 at 4:55 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> 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.

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

Re: [PATCH v3 00/21] improve constexpr handling

[Luc Van Oostenryck]

On Fri, Mar 31, 2017 at 12:40 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Fri, Mar 31, 2017 at 4:55 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> 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.
>
> 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.

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