[tarantool-patches] Re: [PATCH 7/8] sql: clean-up affinity from SQL source code

n.pettik korablev at tarantool.org
Tue Feb 5 20:46:22 MSK 2019


>> -                * 15 is 1111 in a binary form.
>> -                * Since all existing types can be fitted in 4 bits
>> -                * (field_type_MAX == 10), it is enough to 'and'
>> -                * p5 with this constant. Node that all other flags
>> -                * that can be stored in p5 are >= 16.
>> +                * Since all existing types can be fitted in 4
>> +                * bits (field_type_MAX == 9), it is enough to
>> +                * 'and' p5 with field mask. Note that values of
>> +                * other flags that can be stored in p5 of these
>> +                * opcodes are >= FIELD_TYPE_MASK.
> 
> They can not be == FIELD_TYPE_MASK. Please, again, do not try to
> write here any constants, including bit counts. A definition named
> MASK is already enough to make it clear, that p5 'and' mask give a
> type.

Ok, I won’t argue. 

> Please, apply the diff below. I made enum field_type_mask an
> anonymous enum, as we usually do with independent constants. Also, I
> used FIELD_TYPE_MASK instead of 15 in static assertion, and removed
> the comment from vdbe.c about what a mask is.

Seems like compilers don’t allow to compare two different enums,
even though their values are defined: 

/tarantool/src/box/field_def.h:91:30: error: comparison between ‘enum field_type’ and ‘enum <anonymous>’ [-Werror=enum-compare]
 static_assert(field_type_MAX <= FIELD_TYPE_MASK, "values of enum field_type "\
 
So I added explicit cast:

diff --git a/src/box/field_def.h b/src/box/field_def.h
index dc7730f9f..f944de9d6 100644
--- a/src/box/field_def.h
+++ b/src/box/field_def.h
@@ -88,8 +88,8 @@ enum {
  * For detailed explanation see context of OP_Eq, OP_Lt etc
  * opcodes in vdbe.c.
  */
-static_assert(field_type_MAX <= FIELD_TYPE_MASK, "values of enum field_type "\
-             "should fit into 4 bits of VdbeOp.p5");
+static_assert((int) field_type_MAX <= (int) FIELD_TYPE_MASK,
+             "values of enum field_type should fit into 4 bits of VdbeOp.p5”);

The rest of diff applied as is.





More information about the Tarantool-patches mailing list