* [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) ®Free2); 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, ®Free1); @@ -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, ®Free1); @@ -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) ®Free2); 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, ®Free1); @@ -4952,25 +4981,31 @@ sqlExprIfFalse(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull) ®Free2); 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
* [tarantool-patches] Re: [PATCH] sql: Change token/opcode relation assertions to static 2019-08-23 15:49 [tarantool-patches] [PATCH] sql: Change token/opcode relation assertions to static Alexey Zavodchenkov @ 2019-08-27 16:40 ` n.pettik 2019-08-30 15:47 ` [tarantool-patches] [PATCH 0/2] " Alexey Zavodchenkov 0 siblings, 1 reply; 6+ messages in thread From: n.pettik @ 2019-08-27 16:40 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 Hello, Nit: please, put links to issue and branch below delimiter --- Unfortunately, I failed to apply your patch neither using git am nor git apply. Make sure that you are using git format-patch utility and don’t manually change patch context. > --- > 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) > ®Free2); > 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”); Nit: I’d group and move out of function all these assertion to the one place. For instance, asserts in sqlExprCodeTarget() and sqlExprIfTrue() verify that token TK_XX corresponds to OP_Xx opcode. Hence let's move them, before definition of sqlExprCodeTarget(). The same applies for group of assertions in VDBE verifying relative opcode positions and in query planner (whereexpr.c) checking relative token positions. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] [PATCH 0/2] Re: [PATCH] sql: Change token/opcode relation assertions to static 2019-08-27 16:40 ` [tarantool-patches] " n.pettik @ 2019-08-30 15:47 ` 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 0 siblings, 2 replies; 6+ messages in thread From: Alexey Zavodchenkov @ 2019-08-30 15:47 UTC (permalink / raw) To: tarantool-patches; +Cc: Alexey Zavodchenkov Thank you for the review! I'm resending the intial patch and changes you suggested as the separate patch. Branch: https://github.com/Separador/tarantool/tree/gh-4240 Alexey Zavodchenkov (2): sql: Change token/opcode relative positions assertion to static Group assertions and move them out of functions src/box/sql/expr.c | 60 +++++++++++++++++------------------------ src/box/sql/vdbe.c | 29 +++++++++++--------- src/box/sql/whereexpr.c | 19 +++++++------ 3 files changed, 50 insertions(+), 58 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] [PATCH 1/2] sql: Change token/opcode relative positions assertion to static 2019-08-30 15:47 ` [tarantool-patches] [PATCH 0/2] " Alexey Zavodchenkov @ 2019-08-30 15:48 ` Alexey Zavodchenkov 2019-08-30 15:48 ` [tarantool-patches] [PATCH 2/2] Group assertions and move them out of functions Alexey Zavodchenkov 1 sibling, 0 replies; 6+ messages in thread From: Alexey Zavodchenkov @ 2019-08-30 15:48 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 --- 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) ®Free2); 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, ®Free1); @@ -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, ®Free1); @@ -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) ®Free2); 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, ®Free1); @@ -4952,25 +4981,31 @@ sqlExprIfFalse(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull) ®Free2); 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
* [tarantool-patches] [PATCH 2/2] Group assertions and move them out of functions 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 ` Alexey Zavodchenkov 2019-09-10 23:14 ` [tarantool-patches] " Nikita Pettik 1 sibling, 1 reply; 6+ messages in thread From: Alexey Zavodchenkov @ 2019-08-30 15:48 UTC (permalink / raw) To: tarantool-patches; +Cc: Alexey Zavodchenkov --- src/box/sql/expr.c | 95 +++++++++++------------------------------ src/box/sql/vdbe.c | 40 +++++++---------- src/box/sql/whereexpr.c | 28 ++++-------- 3 files changed, 50 insertions(+), 113 deletions(-) diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 668562896..de356d29b 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -3669,6 +3669,31 @@ exprCodeVector(Parse * pParse, Expr * p, int *piFreeable) return iResult; } +/* + * static asserts for sqlExprCodeTarget, sqlExprIfTrue and sqlExprIfFalse + */ +static_assert(TK_LT == OP_Lt, "inconsistent token/opcode definition"); +static_assert(TK_LE == OP_Le, "inconsistent token/opcode definition"); +static_assert(TK_GT == OP_Gt, "inconsistent token/opcode definition"); +static_assert(TK_GE == OP_Ge, "inconsistent token/opcode definition"); +static_assert(TK_EQ == OP_Eq, "inconsistent token/opcode definition"); +static_assert(TK_NE == OP_Ne, "inconsistent token/opcode definition"); +static_assert(TK_AND == OP_And, "inconsistent token/opcode definition"); +static_assert(TK_OR == OP_Or, "inconsistent token/opcode definition"); +static_assert(TK_PLUS == OP_Add, "inconsistent token/opcode definition"); +static_assert(TK_MINUS == OP_Subtract, "inconsistent token/opcode definition"); +static_assert(TK_REM == OP_Remainder, "inconsistent token/opcode definition"); +static_assert(TK_BITAND == OP_BitAnd, "inconsistent token/opcode definition"); +static_assert(TK_BITOR == OP_BitOr, "inconsistent token/opcode definition"); +static_assert(TK_SLASH == OP_Divide, "inconsistent token/opcode definition"); +static_assert(TK_LSHIFT == OP_ShiftLeft, "inconsistent token/opcode definition"); +static_assert(TK_RSHIFT == OP_ShiftRight, "inconsistent token/opcode definition"); +static_assert(TK_CONCAT == OP_Concat, "inconsistent token/opcode definition"); +static_assert(TK_BITNOT == OP_BitNot, "inconsistent token/opcode definition"); +static_assert(TK_NOT == OP_Not, "inconsistent token/opcode definition"); +static_assert(TK_ISNULL == OP_IsNull, "inconsistent token/opcode definition"); +static_assert(TK_NOTNULL == OP_NotNull, "inconsistent token/opcode definition"); + /* * Generate code into the current Vdbe to evaluate the given * expression. Attempt to store the results in register "target". @@ -3839,28 +3864,16 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) ®Free2); codeCompare(pParse, pLeft, pExpr->pRight, op, r1, r2, inReg, SQL_STOREP2); - static_assert(TK_LT == OP_Lt, - "inconsistent token/opcode definition"); testcase(op == OP_Lt); VdbeCoverageIf(v, op == OP_Lt); - static_assert(TK_LE == OP_Le, - "inconsistent token/opcode definition"); testcase(op == OP_Le); VdbeCoverageIf(v, op == OP_Le); - static_assert(TK_GT == OP_Gt, - "inconsistent token/opcode definition"); testcase(op == OP_Gt); VdbeCoverageIf(v, op == OP_Gt); - static_assert(TK_GE == OP_Ge, - "inconsistent token/opcode definition"); testcase(op == OP_Ge); VdbeCoverageIf(v, op == OP_Ge); - static_assert(TK_EQ == OP_Eq, - "inconsistent token/opcode definition"); testcase(op == OP_Eq); VdbeCoverageIf(v, op == OP_Eq); - static_assert(TK_NE == OP_Ne, - "inconsistent token/opcode definition"); testcase(op == OP_Ne); VdbeCoverageIf(v, op == OP_Ne); testcase(regFree1 == 0); @@ -3880,38 +3893,16 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) case TK_LSHIFT: case TK_RSHIFT: case TK_CONCAT:{ - static_assert(TK_AND == OP_And, - "inconsistent token/opcode definition"); testcase(op == TK_AND); - static_assert(TK_OR == OP_Or, - "inconsistent token/opcode definition"); testcase(op == TK_OR); - static_assert(TK_PLUS == OP_Add, - "inconsistent token/opcode definition"); testcase(op == TK_PLUS); - static_assert(TK_MINUS == OP_Subtract, - "inconsistent token/opcode definition"); testcase(op == TK_MINUS); - static_assert(TK_REM == OP_Remainder, - "inconsistent token/opcode definition"); testcase(op == TK_REM); - static_assert(TK_BITAND == OP_BitAnd, - "inconsistent token/opcode definition"); testcase(op == TK_BITAND); - static_assert(TK_BITOR == OP_BitOr, - "inconsistent token/opcode definition"); testcase(op == TK_BITOR); - static_assert(TK_SLASH == OP_Divide, - "inconsistent token/opcode definition"); testcase(op == TK_SLASH); - static_assert(TK_LSHIFT == OP_ShiftLeft, - "inconsistent token/opcode definition"); testcase(op == TK_LSHIFT); - static_assert(TK_RSHIFT == OP_ShiftRight, - "inconsistent token/opcode definition"); testcase(op == TK_RSHIFT); - static_assert(TK_CONCAT == OP_Concat, - "inconsistent token/opcode definition"); testcase(op == TK_CONCAT); r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, ®Free1); @@ -3948,11 +3939,7 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) } case TK_BITNOT: case TK_NOT:{ - static_assert(TK_BITNOT == OP_BitNot, - "inconsistent token/opcode definition"); testcase(op == TK_BITNOT); - static_assert(TK_NOT == OP_Not, - "inconsistent token/opcode definition"); testcase(op == TK_NOT); r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, ®Free1); @@ -3963,11 +3950,7 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) case TK_ISNULL: case TK_NOTNULL:{ int addr; - static_assert(TK_ISNULL == OP_IsNull, - "inconsistent token/opcode definition"); testcase(op == TK_ISNULL); - 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, @@ -4780,31 +4763,19 @@ sqlExprIfTrue(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull) ®Free2); codeCompare(pParse, pExpr->pLeft, pExpr->pRight, op, r1, r2, dest, jumpIfNull); - static_assert(TK_LT == OP_Lt, - "inconsistent token/opcode definition"); testcase(op == OP_Lt); VdbeCoverageIf(v, op == OP_Lt); - static_assert(TK_LE == OP_Le, - "inconsistent token/opcode definition"); testcase(op == OP_Le); VdbeCoverageIf(v, op == OP_Le); - static_assert(TK_GT == OP_Gt, - "inconsistent token/opcode definition"); testcase(op == OP_Gt); VdbeCoverageIf(v, op == OP_Gt); - static_assert(TK_GE == OP_Ge, - "inconsistent token/opcode definition"); testcase(op == OP_Ge); VdbeCoverageIf(v, op == OP_Ge); - 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); - static_assert(TK_NE == OP_Ne, - "inconsistent token/opcode definition"); testcase(op == OP_Ne); VdbeCoverageIf(v, op == OP_Ne && jumpIfNull == SQL_NULLEQ); @@ -4816,11 +4787,7 @@ sqlExprIfTrue(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull) } case TK_ISNULL: case TK_NOTNULL:{ - static_assert(TK_ISNULL == OP_IsNull, - "inconsistent token/opcode definition"); testcase(op == TK_ISNULL); - static_assert(TK_NOTNULL == OP_NotNull, - "inconsistent token/opcode definition"); testcase(op == TK_NOTNULL); r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, ®Free1); @@ -4981,31 +4948,19 @@ sqlExprIfFalse(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull) ®Free2); codeCompare(pParse, pExpr->pLeft, pExpr->pRight, op, r1, r2, dest, jumpIfNull); - static_assert(TK_LT == OP_Lt, - "inconsistent token/opcode definition"); testcase(op == OP_Lt); VdbeCoverageIf(v, op == OP_Lt); - static_assert(TK_LE == OP_Le, - "inconsistent token/opcode definition"); testcase(op == OP_Le); VdbeCoverageIf(v, op == OP_Le); - static_assert(TK_GT == OP_Gt, - "inconsistent token/opcode definition"); testcase(op == OP_Gt); VdbeCoverageIf(v, op == OP_Gt); - static_assert(TK_GE == OP_Ge, - "inconsistent token/opcode definition"); testcase(op == OP_Ge); VdbeCoverageIf(v, op == OP_Ge); - 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); - 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 8560e51a1..414b89272 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -743,6 +743,20 @@ vdbe_field_ref_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno, return 0; } +/* + * static asserts for sqlVdbeExec + */ +static_assert(OP_ShiftRight==OP_ShiftLeft+1, "inconsistent opcode definition"); +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_SeekGE+1, "inconsistent opcode definition"); +static_assert((OP_SeekLE & 0x0001)==(OP_SeekGT & 0x0001), + "inconsistent opcode definition"); +static_assert((OP_SeekLT & 0x0001)==(OP_SeekGE & 0x0001), + "inconsistent opcode definition"); +static_assert((OP_IdxLE&1)==(OP_IdxLT&1) && (OP_IdxGE&1)==(OP_IdxGT&1), + "inconsistent opcode definition"); + /* * Execute as much of a VDBE program as we can. * This is the core of sql_step(). @@ -1847,8 +1861,6 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ /* If shifting by a negative amount, shift in the other direction */ if (iB<0) { - static_assert(OP_ShiftRight==OP_ShiftLeft+1, - "inconsistent opcode definition"); op = 2*OP_ShiftLeft + 1 - op; iB = iB>(-64) ? -iB : 64; } @@ -3281,12 +3293,6 @@ case OP_SeekGT: { /* jump, in3 */ pC = p->apCsr[pOp->p1]; assert(pC!=0); assert(pC->eCurType==CURTYPE_TARANTOOL); - 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; @@ -3355,26 +3361,14 @@ case OP_SeekGT: { /* jump, in3 */ * (x > 4.9) -> (x >= 5) * (x <= 4.9) -> (x < 5) */ - if (pIn3->u.r<(double)iKey) { - 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 (pIn3->u.r < (double)iKey) { if ((oc & 0x0001)==(OP_SeekGT & 0x0001)) oc--; } /* If the approximation iKey is smaller than the actual real search * term, substitute <= for < and > for >=. */ - else if (pIn3->u.r>(double)iKey) { - 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"); + else if (pIn3->u.r > (double)iKey) { if ((oc & 0x0001)==(OP_SeekLT & 0x0001)) oc++; } } @@ -4539,8 +4533,6 @@ 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); - 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 2d29c0bc7..3b7d51f80 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -124,6 +124,15 @@ whereClauseInsert(WhereClause * pWC, Expr * p, u16 wtFlags) return idx; } +/* + * static asserts for allowedOp, exprCommute, exprMightBeIndexed + */ +static_assert(TK_GE == TK_EQ + 4, "inconsistent token definition"); +static_assert(TK_LT == TK_GT + 2, "inconsistent token definition"); +static_assert(TK_LE == TK_GT + 1, "inconsistent token definition"); +static_assert(TK_GE == TK_LE + 2, "inconsistent token definition"); +static_assert(TK_IN < TK_GE, "inconsistent token definition"); + /* * Return TRUE if the given operator is one of the operators that is * allowed for an indexable WHERE clause term. The allowed operators are @@ -132,14 +141,6 @@ whereClauseInsert(WhereClause * pWC, Expr * p, u16 wtFlags) static int allowedOp(int op) { - 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; } @@ -190,14 +191,6 @@ exprCommute(Parse * pParse, Expr * pExpr) } SWAP(pExpr->pRight, pExpr->pLeft); if (pExpr->op >= TK_GT) { - 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; } @@ -953,9 +946,6 @@ exprMightBeIndexed(int op, /* The specific comparison operator */ * inequality constraint (>, <, >= or <=), perform the processing * on the first element of the vector. */ - 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
* [tarantool-patches] Re: [PATCH 2/2] Group assertions and move them out of functions 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 ` Nikita Pettik 0 siblings, 0 replies; 6+ messages in thread From: Nikita Pettik @ 2019-09-10 23:14 UTC (permalink / raw) To: tarantool-patches; +Cc: Alexey Zavodchenkov On Fri, Aug 30, 2019 at 06:48:01PM +0300, Alexey Zavodchenkov wrote: Hi. Sorry for quite delayed answer. Firstly, don't put your fixes at a separate commit: just amend in case of commit (git commit --amend) or squash in case of interactive rebase. Then, if you send next patch version bump version passing --subject-prefix='PATCH v2' option to format-patch. Or, you can simply answer to the review letter with provided fixes (i.e. simple inline diff relared to each reviewer's comment). Read details here: https://www.tarantool.io/en/doc/2.2/dev_guide/developer_guidelines/#how-to-submit-a-patch-for-review Below a few codestyle nits and general comments. > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index 668562896..de356d29b 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -3669,6 +3669,31 @@ exprCodeVector(Parse * pParse, Expr * p, int *piFreeable) > return iResult; > } > > +/* > + * static asserts for sqlExprCodeTarget, sqlExprIfTrue and sqlExprIfFalse > + */ Nit: comments in global scope start from double stars: /** * Comment starts from capital letter and ends with a dot. It must not * exceed 72 characters per line. What is more, we don't write comments * just for the sake of commenting - it should explain something. In * this particular case I'd deliver this information: * * Make sure that opcode and token definitions are placed in the correct * order: value of opcode representing unary or binary operation must * be equal to values of corresponding opcode implementing that * operation. It allows to pass token value to sqlVdbeAddOp*() functions * as an opcode value. It is quite convinient since we can avoid mapping * tokens to opcodes. */ Intead of copy-pasting the same string several times, I'd rather once declare it and re-use (and a bit clarify error message): #define TOKEN_TO_OPCODE_ERR "Token representing binary or unary operator "\ "is not equal to opcode implementing it" static_assert(TK_LT == OP_Lt, TOKEN_TO_OPCODE_ERR); Or you can even use stringizing of macro arguments: #define TOKEN_TO_OPCODE_ERR(token, opcode) "Token "#token" representing ... Then we will get fine error message: /tarantool/src/box/sql/expr.c:497:1: error: static_assert failed "Token TK_LT representing binary or unary operator is not equal to opcode OP_Lt implementing it" static_assert(TK_GT == OP_Lt, TOKEN_TO_OPCODE_ERR(TK_LT, OP_Lt)); I demonstrated you example of good comment to this part of asserts. Please, investigate why do we need other asserts and write the same *good* comments for the other two groups. > +static_assert(TK_LT == OP_Lt, "inconsistent token/opcode definition"); > +static_assert(TK_LE == OP_Le, "inconsistent token/opcode definition"); > +static_assert(TK_GT == OP_Gt, "inconsistent token/opcode definition"); > +static_assert(TK_GE == OP_Ge, "inconsistent token/opcode definition"); > +static_assert(TK_EQ == OP_Eq, "inconsistent token/opcode definition"); > +static_assert(TK_NE == OP_Ne, "inconsistent token/opcode definition"); > +static_assert(TK_AND == OP_And, "inconsistent token/opcode definition"); > +static_assert(TK_OR == OP_Or, "inconsistent token/opcode definition"); > +static_assert(TK_PLUS == OP_Add, "inconsistent token/opcode definition"); > +static_assert(TK_MINUS == OP_Subtract, "inconsistent token/opcode definition"); > +static_assert(TK_REM == OP_Remainder, "inconsistent token/opcode definition"); > +static_assert(TK_BITAND == OP_BitAnd, "inconsistent token/opcode definition"); > +static_assert(TK_BITOR == OP_BitOr, "inconsistent token/opcode definition"); > +static_assert(TK_SLASH == OP_Divide, "inconsistent token/opcode definition"); > +static_assert(TK_LSHIFT == OP_ShiftLeft, "inconsistent token/opcode definition"); > +static_assert(TK_RSHIFT == OP_ShiftRight, "inconsistent token/opcode definition"); > +static_assert(TK_CONCAT == OP_Concat, "inconsistent token/opcode definition"); > +static_assert(TK_BITNOT == OP_BitNot, "inconsistent token/opcode definition"); > +static_assert(TK_NOT == OP_Not, "inconsistent token/opcode definition"); > +static_assert(TK_ISNULL == OP_IsNull, "inconsistent token/opcode definition"); > +static_assert(TK_NOTNULL == OP_NotNull, "inconsistent token/opcode definition"); > + > /* > * Generate code into the current Vdbe to evaluate the given > * expression. Attempt to store the results in register "target". > @@ -3839,28 +3864,16 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > ®Free2); > codeCompare(pParse, pLeft, pExpr->pRight, op, > r1, r2, inReg, SQL_STOREP2); > - static_assert(TK_LT == OP_Lt, > - "inconsistent token/opcode definition"); > testcase(op == OP_Lt); > VdbeCoverageIf(v, op == OP_Lt); > - static_assert(TK_LE == OP_Le, > - "inconsistent token/opcode definition"); > testcase(op == OP_Le); > VdbeCoverageIf(v, op == OP_Le); > - static_assert(TK_GT == OP_Gt, > - "inconsistent token/opcode definition"); > testcase(op == OP_Gt); > VdbeCoverageIf(v, op == OP_Gt); > - static_assert(TK_GE == OP_Ge, > - "inconsistent token/opcode definition"); > testcase(op == OP_Ge); > VdbeCoverageIf(v, op == OP_Ge); > - static_assert(TK_EQ == OP_Eq, > - "inconsistent token/opcode definition"); > testcase(op == OP_Eq); > VdbeCoverageIf(v, op == OP_Eq); > - static_assert(TK_NE == OP_Ne, > - "inconsistent token/opcode definition"); > testcase(op == OP_Ne); > VdbeCoverageIf(v, op == OP_Ne); > testcase(regFree1 == 0); > @@ -3880,38 +3893,16 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > case TK_LSHIFT: > case TK_RSHIFT: > case TK_CONCAT:{ > - static_assert(TK_AND == OP_And, > - "inconsistent token/opcode definition"); > testcase(op == TK_AND); > - static_assert(TK_OR == OP_Or, > - "inconsistent token/opcode definition"); > testcase(op == TK_OR); > - static_assert(TK_PLUS == OP_Add, > - "inconsistent token/opcode definition"); > testcase(op == TK_PLUS); > - static_assert(TK_MINUS == OP_Subtract, > - "inconsistent token/opcode definition"); > testcase(op == TK_MINUS); > - static_assert(TK_REM == OP_Remainder, > - "inconsistent token/opcode definition"); > testcase(op == TK_REM); > - static_assert(TK_BITAND == OP_BitAnd, > - "inconsistent token/opcode definition"); > testcase(op == TK_BITAND); > - static_assert(TK_BITOR == OP_BitOr, > - "inconsistent token/opcode definition"); > testcase(op == TK_BITOR); > - static_assert(TK_SLASH == OP_Divide, > - "inconsistent token/opcode definition"); > testcase(op == TK_SLASH); > - static_assert(TK_LSHIFT == OP_ShiftLeft, > - "inconsistent token/opcode definition"); > testcase(op == TK_LSHIFT); > - static_assert(TK_RSHIFT == OP_ShiftRight, > - "inconsistent token/opcode definition"); > testcase(op == TK_RSHIFT); > - static_assert(TK_CONCAT == OP_Concat, > - "inconsistent token/opcode definition"); > testcase(op == TK_CONCAT); > r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, > ®Free1); > @@ -3948,11 +3939,7 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > } > case TK_BITNOT: > case TK_NOT:{ > - static_assert(TK_BITNOT == OP_BitNot, > - "inconsistent token/opcode definition"); > testcase(op == TK_BITNOT); > - static_assert(TK_NOT == OP_Not, > - "inconsistent token/opcode definition"); > testcase(op == TK_NOT); > r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, > ®Free1); > @@ -3963,11 +3950,7 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > case TK_ISNULL: > case TK_NOTNULL:{ > int addr; > - static_assert(TK_ISNULL == OP_IsNull, > - "inconsistent token/opcode definition"); > testcase(op == TK_ISNULL); > - 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, > @@ -4780,31 +4763,19 @@ sqlExprIfTrue(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull) > ®Free2); > codeCompare(pParse, pExpr->pLeft, pExpr->pRight, op, r1, > r2, dest, jumpIfNull); > - static_assert(TK_LT == OP_Lt, > - "inconsistent token/opcode definition"); > testcase(op == OP_Lt); > VdbeCoverageIf(v, op == OP_Lt); > - static_assert(TK_LE == OP_Le, > - "inconsistent token/opcode definition"); > testcase(op == OP_Le); > VdbeCoverageIf(v, op == OP_Le); > - static_assert(TK_GT == OP_Gt, > - "inconsistent token/opcode definition"); > testcase(op == OP_Gt); > VdbeCoverageIf(v, op == OP_Gt); > - static_assert(TK_GE == OP_Ge, > - "inconsistent token/opcode definition"); > testcase(op == OP_Ge); > VdbeCoverageIf(v, op == OP_Ge); > - 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); > - static_assert(TK_NE == OP_Ne, > - "inconsistent token/opcode definition"); > testcase(op == OP_Ne); > VdbeCoverageIf(v, op == OP_Ne > && jumpIfNull == SQL_NULLEQ); > @@ -4816,11 +4787,7 @@ sqlExprIfTrue(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull) > } > case TK_ISNULL: > case TK_NOTNULL:{ > - static_assert(TK_ISNULL == OP_IsNull, > - "inconsistent token/opcode definition"); > testcase(op == TK_ISNULL); > - static_assert(TK_NOTNULL == OP_NotNull, > - "inconsistent token/opcode definition"); > testcase(op == TK_NOTNULL); > r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, > ®Free1); > @@ -4981,31 +4948,19 @@ sqlExprIfFalse(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull) > ®Free2); > codeCompare(pParse, pExpr->pLeft, pExpr->pRight, op, r1, > r2, dest, jumpIfNull); > - static_assert(TK_LT == OP_Lt, > - "inconsistent token/opcode definition"); > testcase(op == OP_Lt); > VdbeCoverageIf(v, op == OP_Lt); > - static_assert(TK_LE == OP_Le, > - "inconsistent token/opcode definition"); > testcase(op == OP_Le); > VdbeCoverageIf(v, op == OP_Le); > - static_assert(TK_GT == OP_Gt, > - "inconsistent token/opcode definition"); > testcase(op == OP_Gt); > VdbeCoverageIf(v, op == OP_Gt); > - static_assert(TK_GE == OP_Ge, > - "inconsistent token/opcode definition"); > testcase(op == OP_Ge); > VdbeCoverageIf(v, op == OP_Ge); > - 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); > - 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 8560e51a1..414b89272 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -743,6 +743,20 @@ vdbe_field_ref_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno, > return 0; > } > > +/* > + * static asserts for sqlVdbeExec > + */ > +static_assert(OP_ShiftRight==OP_ShiftLeft+1, "inconsistent opcode definition"); > +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_SeekGE+1, "inconsistent opcode definition"); > +static_assert((OP_SeekLE & 0x0001)==(OP_SeekGT & 0x0001), > + "inconsistent opcode definition"); > +static_assert((OP_SeekLT & 0x0001)==(OP_SeekGE & 0x0001), > + "inconsistent opcode definition"); > +static_assert((OP_IdxLE&1)==(OP_IdxLT&1) && (OP_IdxGE&1)==(OP_IdxGT&1), > + "inconsistent opcode definition"); > + > /* > * Execute as much of a VDBE program as we can. > * This is the core of sql_step(). > @@ -1847,8 +1861,6 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ > > /* If shifting by a negative amount, shift in the other direction */ > if (iB<0) { > - static_assert(OP_ShiftRight==OP_ShiftLeft+1, > - "inconsistent opcode definition"); > op = 2*OP_ShiftLeft + 1 - op; > iB = iB>(-64) ? -iB : 64; > } > @@ -3281,12 +3293,6 @@ case OP_SeekGT: { /* jump, in3 */ > pC = p->apCsr[pOp->p1]; > assert(pC!=0); > assert(pC->eCurType==CURTYPE_TARANTOOL); > - 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; > @@ -3355,26 +3361,14 @@ case OP_SeekGT: { /* jump, in3 */ > * (x > 4.9) -> (x >= 5) > * (x <= 4.9) -> (x < 5) > */ > - if (pIn3->u.r<(double)iKey) { > - 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 (pIn3->u.r < (double)iKey) { > if ((oc & 0x0001)==(OP_SeekGT & 0x0001)) oc--; > } > > /* If the approximation iKey is smaller than the actual real search > * term, substitute <= for < and > for >=. > */ > - else if (pIn3->u.r>(double)iKey) { > - 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"); > + else if (pIn3->u.r > (double)iKey) { > if ((oc & 0x0001)==(OP_SeekLT & 0x0001)) oc++; > } > } > @@ -4539,8 +4533,6 @@ 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); > - 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 2d29c0bc7..3b7d51f80 100644 > --- a/src/box/sql/whereexpr.c > +++ b/src/box/sql/whereexpr.c > @@ -124,6 +124,15 @@ whereClauseInsert(WhereClause * pWC, Expr * p, u16 wtFlags) > return idx; > } > > +/* > + * static asserts for allowedOp, exprCommute, exprMightBeIndexed > + */ > +static_assert(TK_GE == TK_EQ + 4, "inconsistent token definition"); > +static_assert(TK_LT == TK_GT + 2, "inconsistent token definition"); > +static_assert(TK_LE == TK_GT + 1, "inconsistent token definition"); > +static_assert(TK_GE == TK_LE + 2, "inconsistent token definition"); > +static_assert(TK_IN < TK_GE, "inconsistent token definition"); > + > /* > * Return TRUE if the given operator is one of the operators that is > * allowed for an indexable WHERE clause term. The allowed operators are > @@ -132,14 +141,6 @@ whereClauseInsert(WhereClause * pWC, Expr * p, u16 wtFlags) > static int > allowedOp(int op) > { > - 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; > } > > @@ -190,14 +191,6 @@ exprCommute(Parse * pParse, Expr * pExpr) > } > SWAP(pExpr->pRight, pExpr->pLeft); > if (pExpr->op >= TK_GT) { > - 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; > } > @@ -953,9 +946,6 @@ exprMightBeIndexed(int op, /* The specific comparison operator */ > * inequality constraint (>, <, >= or <=), perform the processing > * on the first element of the vector. > */ > - 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