From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Alexey Zavodchenkov <zavodchen@gmail.com> Subject: [tarantool-patches] Re: [PATCH] sql: Change token/opcode relation assertions to static Date: Tue, 27 Aug 2019 19:40:50 +0300 [thread overview] Message-ID: <A2C8E320-3DD6-4061-937A-A86776F527C4@tarantool.org> (raw) In-Reply-To: <20190823154958.20837-1-zavodchen@gmail.com> > 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.
next prev parent reply other threads:[~2019-08-27 16:40 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-23 15:49 [tarantool-patches] " Alexey Zavodchenkov 2019-08-27 16:40 ` n.pettik [this message] 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
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=A2C8E320-3DD6-4061-937A-A86776F527C4@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=zavodchen@gmail.com \ --subject='[tarantool-patches] Re: [PATCH] sql: Change token/opcode relation assertions to static' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox