[HarfBuzz] enum issue

[Behdad Esfahbod]

On 14-03-03 09:24 PM, Jonathan Kew wrote:
> On 4/3/14 03:52, Behdad Esfahbod wrote:
>> On 14-03-01 08:38 AM, Werner LEMBERG wrote:
>>>
>>> Compiling with -ansi using
>>>
>>>    i686-apple-darwin11-llvm-gcc-4.2 (GCC) 4.2.1
>>>    (Based on Apple Inc. build 5658) (LLVM build 2336.11.00)
>>>
>>> I get the following warning:
>>>
>>>    hb-common.h:280: warning:
>>>      ISO C restricts enumerator values to range of 'int'
>>>
>>> Nothing really serious, but probably worthwhile to mention...
>>
>> Humm.  I was under the impression that the compiler chooses unsigned int if
>> necessary.  Don't know.  We definitely want hb_tag_t to be equivalent to
>> unsigned.  Jonathan, WDYT?
>>
> 
> This isn't a problem in C++, AIUI, but only for C clients that include the
> header. ISO C says:
> 
>   Each enumerated type shall be compatible with char, a signed
>   integer type, or an unsigned integer type. The choice of type is
>   implementation-defined, but shall be capable of representing the
>   values of all the members of the enumeration.
> 
> which is fine, but it also says:
> 
>   The expression that defines the value of an enumeration constant
>   shall be an integer constant expression that has a value
>   representable as an int.
> 
> That's what leads to the pedantic warning: we have HB_TAG_MAX defined as the
> hb_tag_t value of 0xffffffff, and hb_tag_t is uint32_t, so this is the integer
> 4294967295, which is not representable as an int.
> 
> If we change the last value in the hb_script_t enumeration to
> 
>   /*---*/ _HB_SCRIPT_MAX_VALUE = (int) HB_TAG_MAX
> 
> i.e. add an explicit cast, then I believe we shouldn't get a warning. However,
> this might mean the compiler will choose to represent the hb_script_t
> enumeration as int instead of unsigned int (because one of its values will be
> negative). But AFAICT this should be harmless.

That negates the reason we added that line to begin with, which was to ensure
all hb_tag_t's fit in hb_script_t.  How about:

  _HB_SCRIPT_DUMMY1 = 0x7FFFFFFF,
  _HB_SCRIPT_DUMMY2

The idea being that DUMMY2 gets value of 0x80000000, ensuring that hb_script_t
can represent up to 0xFFFFFFFF.

On 14-03-04 12:05 AM, Richard Wordingham wrote:
> Consequently, shouldn't HB_TAG_MAX have the value of 0x7E7E7E7E, which
> is a positive signed 32-bit integer and therefore avoids the problem
> in the first place.

But, we don't want undefined behavior if a hb_tag_t that has non-ASCII is
passed to hb_script_from_tag().  Ie., we cannot rely on that promise.

-- 
behdad
http://behdad.org/