From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 8E853306EA for ; Tue, 4 Jun 2019 15:34:49 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dMvsAanSyC1P for ; Tue, 4 Jun 2019 15:34:49 -0400 (EDT) Received: from smtp18.mail.ru (smtp18.mail.ru [94.100.176.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 4A83C30639 for ; Tue, 4 Jun 2019 15:34:49 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 4/9] sql: rework diag_set() in OP_Halt References: <116d34dc76fbe262015c0079bba2dc00f6a40c8c.1559043251.git.imeevma@gmail.com> <0208a503-1a04-fb78-cf8b-563185c5ab42@tarantool.org> <2f731c49-b8c4-f25b-a0c8-b5dcf382b60c@tarantool.org> From: Vladislav Shpilevoy Message-ID: <0ac8fb6a-b06f-5c36-6bab-11dbcb128484@tarantool.org> Date: Tue, 4 Jun 2019 21:34:45 +0200 MIME-Version: 1.0 In-Reply-To: <2f731c49-b8c4-f25b-a0c8-b5dcf382b60c@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, Imeev Mergen On 03/06/2019 11:41, Imeev Mergen wrote: > Hi! Thank you for the review! My answers below. Also, I think I'm > going to drop this patch, since I plan to move the error code from > p5 to p2 in the next patch. > > On 6/2/19 7:35 PM, Vladislav Shpilevoy wrote: >> >> On 28/05/2019 14:39, imeevma@tarantool.org wrote: >>> Prior to this patch, the way to set Tarantool error in OP_Halt was >>> too universal. It was possible to set a description of the error >>> that does not match its errcode. >> There was a concrete reason, why it was possible - because different >> error codes have different arguments of various types, and the only way >> to set an error at parsing stage is to allow to set arbitrary error >> message to any error code. Without '...', va_arg etc. Besides, we could >> use it to set correct line number and function name in future. Now you >> use diag_set, which restricts us. >> >> So why do you need that patch? We will need to revert it when an error >> appears requiring more than one argument, or an argument of not >> const char * type. That will definitely happen. > For some reason, manually creating an error message looks a bit > wrong to me. How about creating a new macro to create such > messages, for example, diag_prepare(), which takes the same > arguments as diag_set() and returns the complete error message? Perhaps. Sounds good to me. You can do it, if you want. But I would suggest name "diag_prepare_msg", because it won't create a struct error object. Only create a message.