Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: Change token/opcode relation assertions to static
@ 2019-08-23 15:49 Alexey Zavodchenkov
  2019-08-27 16:40 ` [tarantool-patches] " n.pettik
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Zavodchenkov @ 2019-08-23 15:49 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Alexey Zavodchenkov

There are several token/opcode relative positions assertions in the sql codebase
and those tokens/opcodes are defined before the source compilation.
So it is possible to change those assertions to static ones.

Closes: #4240

Issue: https://github.com/tarantool/tarantool/issues/4240
Commit in the fork: https://github.com/Separador/tarantool/commit/d36d7c8080
---
 src/box/sql/expr.c      | 105 ++++++++++++++++++++++++++--------------
 src/box/sql/vdbe.c      |  33 ++++++++-----
 src/box/sql/whereexpr.c |  29 +++++++----
 3 files changed, 111 insertions(+), 56 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 1f9d91705..668562896 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -3839,22 +3839,28 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 							 &regFree2);
 				codeCompare(pParse, pLeft, pExpr->pRight, op,
 					    r1, r2, inReg, SQL_STOREP2);
-				assert(TK_LT == OP_Lt);
+				static_assert(TK_LT == OP_Lt,
+					"inconsistent token/opcode definition");
 				testcase(op == OP_Lt);
 				VdbeCoverageIf(v, op == OP_Lt);
-				assert(TK_LE == OP_Le);
+				static_assert(TK_LE == OP_Le,
+					"inconsistent token/opcode definition");
 				testcase(op == OP_Le);
 				VdbeCoverageIf(v, op == OP_Le);
-				assert(TK_GT == OP_Gt);
+				static_assert(TK_GT == OP_Gt,
+					"inconsistent token/opcode definition");
 				testcase(op == OP_Gt);
 				VdbeCoverageIf(v, op == OP_Gt);
-				assert(TK_GE == OP_Ge);
+				static_assert(TK_GE == OP_Ge,
+					"inconsistent token/opcode definition");
 				testcase(op == OP_Ge);
 				VdbeCoverageIf(v, op == OP_Ge);
-				assert(TK_EQ == OP_Eq);
+				static_assert(TK_EQ == OP_Eq,
+					"inconsistent token/opcode definition");
 				testcase(op == OP_Eq);
 				VdbeCoverageIf(v, op == OP_Eq);
-				assert(TK_NE == OP_Ne);
+				static_assert(TK_NE == OP_Ne,
+					"inconsistent token/opcode definition");
 				testcase(op == OP_Ne);
 				VdbeCoverageIf(v, op == OP_Ne);
 				testcase(regFree1 == 0);
@@ -3874,27 +3880,38 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 	case TK_LSHIFT:
 	case TK_RSHIFT:
 	case TK_CONCAT:{
-			assert(TK_AND == OP_And);
+			static_assert(TK_AND == OP_And,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_AND);
-			assert(TK_OR == OP_Or);
+			static_assert(TK_OR == OP_Or,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_OR);
-			assert(TK_PLUS == OP_Add);
+			static_assert(TK_PLUS == OP_Add,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_PLUS);
-			assert(TK_MINUS == OP_Subtract);
+			static_assert(TK_MINUS == OP_Subtract,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_MINUS);
-			assert(TK_REM == OP_Remainder);
+			static_assert(TK_REM == OP_Remainder,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_REM);
-			assert(TK_BITAND == OP_BitAnd);
+			static_assert(TK_BITAND == OP_BitAnd,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_BITAND);
-			assert(TK_BITOR == OP_BitOr);
+			static_assert(TK_BITOR == OP_BitOr,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_BITOR);
-			assert(TK_SLASH == OP_Divide);
+			static_assert(TK_SLASH == OP_Divide,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_SLASH);
-			assert(TK_LSHIFT == OP_ShiftLeft);
+			static_assert(TK_LSHIFT == OP_ShiftLeft,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_LSHIFT);
-			assert(TK_RSHIFT == OP_ShiftRight);
+			static_assert(TK_RSHIFT == OP_ShiftRight,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_RSHIFT);
-			assert(TK_CONCAT == OP_Concat);
+			static_assert(TK_CONCAT == OP_Concat,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_CONCAT);
 			r1 = sqlExprCodeTemp(pParse, pExpr->pLeft,
 						 &regFree1);
@@ -3931,9 +3948,11 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 		}
 	case TK_BITNOT:
 	case TK_NOT:{
-			assert(TK_BITNOT == OP_BitNot);
+			static_assert(TK_BITNOT == OP_BitNot,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_BITNOT);
-			assert(TK_NOT == OP_Not);
+			static_assert(TK_NOT == OP_Not,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_NOT);
 			r1 = sqlExprCodeTemp(pParse, pExpr->pLeft,
 						 &regFree1);
@@ -3944,9 +3963,11 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 	case TK_ISNULL:
 	case TK_NOTNULL:{
 			int addr;
-			assert(TK_ISNULL == OP_IsNull);
+			static_assert(TK_ISNULL == OP_IsNull,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_ISNULL);
-			assert(TK_NOTNULL == OP_NotNull);
+			static_assert(TK_NOTNULL == OP_NotNull,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_NOTNULL);
 			sqlVdbeAddOp2(v, OP_Bool, true, target);
 			r1 = sqlExprCodeTemp(pParse, pExpr->pLeft,
@@ -4759,25 +4780,31 @@ sqlExprIfTrue(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull)
 						 &regFree2);
 			codeCompare(pParse, pExpr->pLeft, pExpr->pRight, op, r1,
 				    r2, dest, jumpIfNull);
-			assert(TK_LT == OP_Lt);
+			static_assert(TK_LT == OP_Lt,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Lt);
 			VdbeCoverageIf(v, op == OP_Lt);
-			assert(TK_LE == OP_Le);
+			static_assert(TK_LE == OP_Le,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Le);
 			VdbeCoverageIf(v, op == OP_Le);
-			assert(TK_GT == OP_Gt);
+			static_assert(TK_GT == OP_Gt,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Gt);
 			VdbeCoverageIf(v, op == OP_Gt);
-			assert(TK_GE == OP_Ge);
+			static_assert(TK_GE == OP_Ge,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Ge);
 			VdbeCoverageIf(v, op == OP_Ge);
-			assert(TK_EQ == OP_Eq);
+			static_assert(TK_EQ == OP_Eq,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Eq);
 			VdbeCoverageIf(v, op == OP_Eq
 				       && jumpIfNull == SQL_NULLEQ);
 			VdbeCoverageIf(v, op == OP_Eq
 				       && jumpIfNull != SQL_NULLEQ);
-			assert(TK_NE == OP_Ne);
+			static_assert(TK_NE == OP_Ne,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Ne);
 			VdbeCoverageIf(v, op == OP_Ne
 				       && jumpIfNull == SQL_NULLEQ);
@@ -4789,9 +4816,11 @@ sqlExprIfTrue(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull)
 		}
 	case TK_ISNULL:
 	case TK_NOTNULL:{
-			assert(TK_ISNULL == OP_IsNull);
+			static_assert(TK_ISNULL == OP_IsNull,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_ISNULL);
-			assert(TK_NOTNULL == OP_NotNull);
+			static_assert(TK_NOTNULL == OP_NotNull,
+				"inconsistent token/opcode definition");
 			testcase(op == TK_NOTNULL);
 			r1 = sqlExprCodeTemp(pParse, pExpr->pLeft,
 						 &regFree1);
@@ -4952,25 +4981,31 @@ sqlExprIfFalse(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull)
 						 &regFree2);
 			codeCompare(pParse, pExpr->pLeft, pExpr->pRight, op, r1,
 				    r2, dest, jumpIfNull);
-			assert(TK_LT == OP_Lt);
+			static_assert(TK_LT == OP_Lt,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Lt);
 			VdbeCoverageIf(v, op == OP_Lt);
-			assert(TK_LE == OP_Le);
+			static_assert(TK_LE == OP_Le,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Le);
 			VdbeCoverageIf(v, op == OP_Le);
-			assert(TK_GT == OP_Gt);
+			static_assert(TK_GT == OP_Gt,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Gt);
 			VdbeCoverageIf(v, op == OP_Gt);
-			assert(TK_GE == OP_Ge);
+			static_assert(TK_GE == OP_Ge,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Ge);
 			VdbeCoverageIf(v, op == OP_Ge);
-			assert(TK_EQ == OP_Eq);
+			static_assert(TK_EQ == OP_Eq,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Eq);
 			VdbeCoverageIf(v, op == OP_Eq
 				       && jumpIfNull != SQL_NULLEQ);
 			VdbeCoverageIf(v, op == OP_Eq
 				       && jumpIfNull == SQL_NULLEQ);
-			assert(TK_NE == OP_Ne);
+			static_assert(TK_NE == OP_Ne,
+				"inconsistent token/opcode definition");
 			testcase(op == OP_Ne);
 			VdbeCoverageIf(v, op == OP_Ne
 				       && jumpIfNull != SQL_NULLEQ);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index dced8fc73..8560e51a1 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1847,7 +1847,8 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 
 		/* If shifting by a negative amount, shift in the other direction */
 		if (iB<0) {
-			assert(OP_ShiftRight==OP_ShiftLeft+1);
+			static_assert(OP_ShiftRight==OP_ShiftLeft+1,
+				"inconsistent opcode definition");
 			op = 2*OP_ShiftLeft + 1 - op;
 			iB = iB>(-64) ? -iB : 64;
 		}
@@ -3280,9 +3281,12 @@ case OP_SeekGT: {       /* jump, in3 */
 	pC = p->apCsr[pOp->p1];
 	assert(pC!=0);
 	assert(pC->eCurType==CURTYPE_TARANTOOL);
-	assert(OP_SeekLE == OP_SeekLT+1);
-	assert(OP_SeekGE == OP_SeekLT+2);
-	assert(OP_SeekGT == OP_SeekLT+3);
+	static_assert(OP_SeekLE == OP_SeekLT+1,
+		"inconsistent opcode definition");
+	static_assert(OP_SeekGE == OP_SeekLT+2,
+		"inconsistent opcode definition");
+	static_assert(OP_SeekGT == OP_SeekLT+3,
+		"inconsistent opcode definition");
 	assert(pC->uc.pCursor!=0);
 	oc = pOp->opcode;
 	eqOnly = 0;
@@ -3352,9 +3356,12 @@ case OP_SeekGT: {       /* jump, in3 */
 			 *        (x <= 4.9)    ->     (x <  5)
 			 */
 			if (pIn3->u.r<(double)iKey) {
-				assert(OP_SeekGE==(OP_SeekGT-1));
-				assert(OP_SeekLT==(OP_SeekLE-1));
-				assert((OP_SeekLE & 0x0001)==(OP_SeekGT & 0x0001));
+				static_assert(OP_SeekGE==(OP_SeekGT-1),
+					"inconsistent opcode definition");
+				static_assert(OP_SeekLT==(OP_SeekLE-1),
+					"inconsistent opcode definition");
+				static_assert((OP_SeekLE & 0x0001)==(OP_SeekGT & 0x0001),
+					"inconsistent opcode definition");
 				if ((oc & 0x0001)==(OP_SeekGT & 0x0001)) oc--;
 			}
 
@@ -3362,9 +3369,12 @@ case OP_SeekGT: {       /* jump, in3 */
 			 * term, substitute <= for < and > for >=.
 			 */
 			else if (pIn3->u.r>(double)iKey) {
-				assert(OP_SeekLE==(OP_SeekLT+1));
-				assert(OP_SeekGT==(OP_SeekGE+1));
-				assert((OP_SeekLT & 0x0001)==(OP_SeekGE & 0x0001));
+				static_assert(OP_SeekLE==(OP_SeekLT+1),
+					"inconsistent opcode definition");
+				static_assert(OP_SeekGT==(OP_SeekGE+1),
+					"inconsistent opcode definition");
+				static_assert((OP_SeekLT & 0x0001)==(OP_SeekGE & 0x0001),
+					"inconsistent opcode definition");
 				if ((oc & 0x0001)==(OP_SeekLT & 0x0001)) oc++;
 			}
 		}
@@ -4529,7 +4539,8 @@ case OP_IdxGE:  {       /* jump */
 	{ int i; for(i=0; i<r.nField; i++) assert(memIsValid(&r.aMem[i])); }
 #endif
 	int res =  tarantoolsqlIdxKeyCompare(pC->uc.pCursor, &r);
-	assert((OP_IdxLE&1)==(OP_IdxLT&1) && (OP_IdxGE&1)==(OP_IdxGT&1));
+	static_assert((OP_IdxLE&1)==(OP_IdxLT&1) && (OP_IdxGE&1)==(OP_IdxGT&1),
+			"inconsistent opcode definition");
 	if ((pOp->opcode&1)==(OP_IdxLT&1)) {
 		assert(pOp->opcode==OP_IdxLE || pOp->opcode==OP_IdxLT);
 		res = -res;
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 8adf6a5f1..2d29c0bc7 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -132,10 +132,14 @@ whereClauseInsert(WhereClause * pWC, Expr * p, u16 wtFlags)
 static int
 allowedOp(int op)
 {
-	assert(TK_GT > TK_EQ && TK_GT < TK_GE);
-	assert(TK_LT > TK_EQ && TK_LT < TK_GE);
-	assert(TK_LE > TK_EQ && TK_LE < TK_GE);
-	assert(TK_GE == TK_EQ + 4);
+	static_assert(TK_GT > TK_EQ && TK_GT < TK_GE,
+		"inconsistent token definition");
+	static_assert(TK_LT > TK_EQ && TK_LT < TK_GE,
+		"inconsistent token definition");
+	static_assert(TK_LE > TK_EQ && TK_LE < TK_GE,
+		"inconsistent token definition");
+	static_assert(TK_GE == TK_EQ + 4,
+		"inconsistent token definition");
 	return op == TK_IN || (op >= TK_EQ && op <= TK_GE) || op == TK_ISNULL;
 }
 
@@ -186,10 +190,14 @@ exprCommute(Parse * pParse, Expr * pExpr)
 	}
 	SWAP(pExpr->pRight, pExpr->pLeft);
 	if (pExpr->op >= TK_GT) {
-		assert(TK_LT == TK_GT + 2);
-		assert(TK_GE == TK_LE + 2);
-		assert(TK_GT > TK_EQ);
-		assert(TK_GT < TK_LE);
+		static_assert(TK_LT == TK_GT + 2,
+			"inconsistent token definition");
+		static_assert(TK_GE == TK_LE + 2,
+			"inconsistent token definition");
+		static_assert(TK_GT > TK_EQ,
+			"inconsistent token definition");
+		static_assert(TK_GT < TK_LE,
+			"inconsistent token definition");
 		assert(pExpr->op >= TK_GT && pExpr->op <= TK_GE);
 		pExpr->op = ((pExpr->op - TK_GT) ^ 2) + TK_GT;
 	}
@@ -945,8 +953,9 @@ exprMightBeIndexed(int op,	/* The specific comparison operator */
 	 * inequality constraint (>, <, >= or <=), perform the processing
 	 * on the first element of the vector.
 	 */
-	assert(TK_GT + 1 == TK_LE && TK_GT + 2 == TK_LT && TK_GT + 3 == TK_GE);
-	assert(TK_IN < TK_GE);
+	static_assert(TK_GT + 1 == TK_LE && TK_GT + 2 == TK_LT && TK_GT + 3 == TK_GE,
+		"inconsistent token definition");
+	static_assert(TK_IN < TK_GE, "inconsistent token definition");
 	assert(op <= TK_GE || op == TK_ISNULL || op == TK_NOTNULL);
 	if (pExpr->op == TK_VECTOR && (op >= TK_GT && op <= TK_GE)) {
 		pExpr = pExpr->x.pList->a[0].pExpr;
-- 
2.21.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-09-10 23:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-23 15:49 [tarantool-patches] [PATCH] sql: Change token/opcode relation assertions to static Alexey Zavodchenkov
2019-08-27 16:40 ` [tarantool-patches] " n.pettik
2019-08-30 15:47   ` [tarantool-patches] [PATCH 0/2] " Alexey Zavodchenkov
2019-08-30 15:48     ` [tarantool-patches] [PATCH 1/2] sql: Change token/opcode relative positions assertion " Alexey Zavodchenkov
2019-08-30 15:48     ` [tarantool-patches] [PATCH 2/2] Group assertions and move them out of functions Alexey Zavodchenkov
2019-09-10 23:14       ` [tarantool-patches] " Nikita Pettik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox