Browse code

core expr eval: fix assoc., commut and 0 adding for +, ==

- fix associativity for the generic plus: because of the non-fixed
argument types (can work with strings integers or both in the
same type), RVE_PLUS_OP is associative _only_ if the operand
types match.
Non-assoc. example: "a" + 1 + "2" (can be "a12" or "a3").
Instead of adding extra checks for the same type, rely on the
optimizer having already replaced RVE_PLUS_OP with RVE_IPLUS_OP
or RVE_CONCAT_OP => RVE_PLUS_OP will be used only when the
operands types are mixed or cannot be determined (e.g. AVP), or
when script optimizations are turned off (-O0).
- fix commutativity for the generic plus. Similar to the above.
Non-commut. example: 1 + "2" == 3, but "2" + 1 == "21".
- fix commutativity for the generic == and !=.
Non. commut. example: 0 == "" , but "" != 0.
Similar fix as above, relying on the optimizer and the type specific
operators: RVE_IEQ_OP, RVE_IDIFF_OP, RVE_STREQ_OP and
RVE_STRDIFF_OP.
- fix $v + 0 / 0 + $v -> $v optimizations. Because of the generic
plus and recent changes that allow int + str, even if the result
is an int (left side operant is integer), 0 + $v can be
different from $v. E.g.: 0+"1" == 1, which is different from
"1".

Andrei Pelinescu-Onciul authored on 28/04/2009 16:34:03
Showing 1 changed files
... ...
@@ -2281,6 +2281,9 @@ static int rve_op_is_assoc(enum rval_expr_op op)
2281 2281
 		case RVE_MINUS_OP:
2282 2282
 			return 0;
2283 2283
 		case RVE_PLUS_OP:
2284
+			/* the generic plus is not assoc, e.g.
2285
+			   "a" + 1 + "2" => "a12" in one case and "a3" in the other */
2286
+			return 0;
2284 2287
 		case RVE_IPLUS_OP:
2285 2288
 		case RVE_CONCAT_OP:
2286 2289
 		case RVE_MUL_OP:
... ...
@@ -2308,7 +2311,7 @@ static int rve_op_is_assoc(enum rval_expr_op op)
2308 2308
 
2309 2309
 
2310 2310
 /** returns true if the operator is commutative. */
2311
-static int rve_op_is_commutative(enum rval_expr_op op, enum rval_type type)
2311
+static int rve_op_is_commutative(enum rval_expr_op op)
2312 2312
 {
2313 2313
 	switch(op){
2314 2314
 		case RVE_NONE_OP:
... ...
@@ -2325,7 +2328,10 @@ static int rve_op_is_commutative(enum rval_expr_op op, enum rval_type type)
2325 2325
 		case RVE_MINUS_OP:
2326 2326
 			return 0;
2327 2327
 		case RVE_PLUS_OP:
2328
-			return type==RV_INT; /* commutative only for INT*/
2328
+			/* non commut. when diff. type 
2329
+			   (e.g 1 + "2" != "2" + 1 ) => non commut. in general
2330
+			   (specific same type versions are covered by IPLUS & CONCAT) */
2331
+			return 0;
2329 2332
 		case RVE_IPLUS_OP:
2330 2333
 		case RVE_MUL_OP:
2331 2334
 		case RVE_BAND_OP:
... ...
@@ -2345,11 +2351,11 @@ static int rve_op_is_commutative(enum rval_expr_op op, enum rval_type type)
2345 2345
 			return 0;
2346 2346
 		case RVE_DIFF_OP:
2347 2347
 		case RVE_EQ_OP:
2348
-#if !defined(UNDEF_EQ_ALWAYS_FALSE) && !defined(UNDEF_EQ_UNDEF_TRUE)
2349
-			return 1;
2350
-#else
2348
+			/* non. commut. in general, only for same type e.g.:
2349
+			   "" == 0  diff. 0 == "" ( "" == "0" and 0 == 0)
2350
+			   same type versions are covered by IEQ, IDIFF, STREQ, STRDIFF
2351
+			 */
2351 2352
 			return 0 /* asymmetrical undef handling */;
2352
-#endif
2353 2353
 	}
2354 2354
 	return 0;
2355 2355
 }
... ...
@@ -2658,8 +2664,19 @@ static int rve_opt_01(struct rval_expr* rve, enum rval_type rve_type)
2658 2658
 			case RVE_PLUS_OP:
2659 2659
 			case RVE_IPLUS_OP:
2660 2660
 				/* we must make sure that this is an int PLUS
2661
-				   (because "foo"+0 is valid => "foo0") */
2662
-				if ((i==0) && ((op==RVE_IPLUS_OP)||(rve_type==RV_INT))){
2661
+				   (because "foo"+0 is valid => "foo0")
2662
+				  Even if overall type is RV_INT, it's still not safe
2663
+				  to optimize a generic PLUS: 0 + $v is not always equivalent
2664
+				  to $v (e.g. 0+"1" ==1 != "1") => optimize only if
2665
+				  IPLUS (always safe since it converts to int first) or
2666
+				  if generic PLUS and result is integer (rve_type) and
2667
+				  expression is of the form $v+ct (and not ct+$v).
2668
+				  TODO: dropping PLUS_OP all together and relying on the
2669
+				   optimizer replacing safe PLUS_OP with IPLUS_OP or CONCAT_OP
2670
+				   will simplify things.
2671
+				 */
2672
+				if ((i==0) && ((op==RVE_IPLUS_OP)||
2673
+							(rve_type==RV_INT && ct_rve==rve->right.rve))){
2663 2674
 					/* $v +  0 -> $v
2664 2675
 					 *  0 + $v -> $v */
2665 2676
 					rve_destroy(ct_rve);
... ...
@@ -2908,7 +2925,7 @@ static int rve_optimize(struct rval_expr* rve)
2908 2908
 								trv->v.s.len, trv->v.s.s, rve->op);
2909 2909
 					ret=1;
2910 2910
 				}else if (rve_is_constant(rve->left.rve->left.rve) &&
2911
-							rve_op_is_commutative(rve->op, type)){
2911
+							rve_op_is_commutative(rve->op)){
2912 2912
 					/* op(op(a, $v), b) => op(op(a, b), $v) */
2913 2913
 					/* rv= op(a, b) */
2914 2914
 					tmp_rve.op=rve->op;
... ...
@@ -2950,7 +2967,7 @@ static int rve_optimize(struct rval_expr* rve)
2950 2950
 			if ((rve->op==rve->right.rve->op) && rve_op_is_assoc(rve->op)){
2951 2951
 				/* op(a, op(...)) */
2952 2952
 				if (rve_is_constant(rve->right.rve->right.rve) &&
2953
-						rve_op_is_commutative(rve->op, type)){
2953
+						rve_op_is_commutative(rve->op)){
2954 2954
 					/* op(a, op($v, b)) => op(op(a, b), $v) */
2955 2955
 					/* rv= op(a, b) */
2956 2956
 					tmp_rve.op=rve->op;