[Piglit] [PATCH] shader_runner: Cast isspace inputs to int to silence warnings.

[Vinson Lee]

On Tue, Feb 21, 2012 at 7:09 AM, Brian Paul <brianp at vmware.com> wrote:
> On 02/20/2012 09:31 PM, Kenneth Graunke wrote:
>>
>> On 02/19/2012 10:45 PM, Vinson Lee wrote:
>>>
>>> Fixes these GCC warnings on Cygwin.
>>> shader_runner.c: In function ‘strcpy_to_space’:
>>> shader_runner.c:202:2: warning: array subscript has type ‘char’
>>> shader_runner.c: In function ‘eat_whitespace’:
>>> shader_runner.c:216:2: warning: array subscript has type ‘char’
>>> shader_runner.c: In function ‘eat_text’:
>>> shader_runner.c:229:2: warning: array subscript has type ‘char’
>>
>>
>> These are really bizarre warnings. We're not doing array subscripting
>> here at all. We're calling isspace() on a char.
>
>
> IIRC, some std C lib implementations use an array of true/false values
> indexed by character to implement isspace(), isalpha(), etc.  Maybe that's
> what's happening here. But still, it's a weird warning.


This is the source from Cygwin.

/usr/include/ctypes.h
    47  #ifndef __cplusplus
    48  /* These macros are intentionally written in a manner that will trigger
    49     a gcc -Wall warning if the user mistakenly passes a 'char' instead
    50     of an int containing an 'unsigned char'.  Note that the sizeof will
    51     always be 1, which is what we want for mapping EOF to
__ctype_ptr__[0];
    52     the use of a raw index inside the sizeof triggers the gcc warning if
    53     __c was of type char, and sizeof masks side effects of the extra __c.
    54     Meanwhile, the real index to __ctype_ptr__+1 must be cast to int,
    55     since isalpha(0x100000001LL) must equal isalpha(1), rather than being
    56     an out-of-bounds reference on a 64-bit machine.  */
    57  #define __ctype_lookup(__c)
((__ctype_ptr__+sizeof(""[__c]))[(int)(__c)])
    58
    59  #define isalpha(__c)    (__ctype_lookup(__c)&(_U|_L))
    60  #define isupper(__c)    ((__ctype_lookup(__c)&(_U|_L))==_U)
    61  #define islower(__c)    ((__ctype_lookup(__c)&(_U|_L))==_L)
    62  #define isdigit(__c)    (__ctype_lookup(__c)&_N)
    63  #define isxdigit(__c)   (__ctype_lookup(__c)&(_X|_N))
    64  #define isspace(__c)    (__ctype_lookup(__c)&_S)
    65  #define ispunct(__c)    (__ctype_lookup(__c)&_P)
    66  #define isalnum(__c)    (__ctype_lookup(__c)&(_U|_L|_N))
    67  #define isprint(__c)    (__ctype_lookup(__c)&(_P|_U|_L|_N|_B))
    68  #define isgraph(__c)    (__ctype_lookup(__c)&(_P|_U|_L|_N))
    69  #define iscntrl(__c)    (__ctype_lookup(__c)&_C)



>
>
>
>> This is perfectly legal, idiomatic, and common practice. Isn't the
>> whole point of isspace() and friends to tell you what class a
>> /character/ is in? Yes, they're specified with ints, but people use
>> chars all the time. They're supposed to be silently promoted.
>>
>> IMO, if the Cygwin compiler can't handle isspace() on chars, it's
>> broken. I would just turn off the warning.
>>
>> My vote on this patch (and the asmparsertest one) would be 'no'.
>
>
> Vinson, do you know if there's a compiler switch to silence this one?