[tarantool-patches] Re: [PATCH v1 4/9] sql: rework diag_set() in OP_Halt

[Vladislav Shpilevoy]

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 at 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.