Re: [PATCH/RFA] Distinguish between EOF and character with value 0xff

[Wizards' Guild <wizardsguild at earthlink dot net>]

Eric,

I am still contemplating. Fundamentally, I like seeing the warnings but I don't think it's worth trying to avoid the toupper/tolower function call (for locale "C" where other tables are supported) by putting a special case at the call site. I think your main interest is in seeing the warnings. With that in mind, I have the following VERY UGLY version of the macro:

#define toupper(__c) ((toupper)((int)&((unsigned char *)0)[__c]))

This seems to have no overhead, produces the desired warning (and no others!), and always calls the function. This technique could be used in place of the original macros where the extended ctype tables are supported. It should still not be used in C++, I think.

It is a little disappointing that the much simpler 0[__c] doesn't work. This expression is syntactically and semantically exactly what we want, but stumbles into another gcc warning. :/

Mike

-----Original Message-----
>From: Eric Blake <ebb9@byu.net>
>Sent: Apr 24, 2009 9:19 AM
>To: Wizards' Guild <wizardsguild@earthlink.net>
>Cc: newlib@sourceware.org
>Subject: Re: [PATCH/RFA] Distinguish between EOF and character with value   0xff
>
>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>According to Wizards' Guild on 4/24/2009 6:54 AM:
>> Eric,
>> 
>> It occurred to me that using __typeof__ causes another potential issue. If the user calls toupper((unsigned)c), the result ends up being unsigned, which could propagate through the surrounding code and cause warnings or unexpected results. In each of these three macros we should ensure the result is int, as the function would be. I would do it this way:
>> 
>> #define toupper(__c) __extension__ ({ __typeof__(__c) __x = (__c); islower(__x) ? (int) __x - 'a' + 'A' : (int) __x; })
>
>Good catch.  You are correct that my switch to __typeof__ breaks
>toupper/tolower without your followup.  But let's resolve this as part of
>resolving how we want toupper to behave...
>
>> 
>> Seems like the `__ctype_ptr__ ==` test would add a lot of overhead; I'd rather see this remain a build-time test. On my primary architecture (nios2) it almost doubles the size of the call site to load the <WHAT> address.
>
>We have three categories for toupper behavior:
>
>!gcc => function call only, because we can't evaluate macro argument twice
>(no change from current state)
>
>gcc && !extended_mb => macro for inlined computation, because there is no
>confusion about "(int) __x - 'a' + 'A'" (no change from current state,
>even with my attached patch33)
>
>gcc && extended_mb => under consideration
>Currently always a function call, with no warning. My attached patch33
>makes it a macro call which adds the QoI warning but still results in the
>function call, without a pointer comparison. Or we could make this
>dependent on __OPTIMIZE_SIZE__ (always function call, perhaps via a
>warning macro) vs. speed (macro call that performs pointer comparison on
>__ctype_ptr__, so that C locale is inlined but other locales are function
>call).
>
>- --
>Don't work too hard, make some time for fun as well!
>
>Eric Blake             ebb9@byu.net
>-----BEGIN PGP SIGNATURE-----
>Version: GnuPG v1.4.9 (Cygwin)
>Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
>Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
>iEYEARECAAYFAknxvEsACgkQ84KuGfSFAYBe3wCgosZvJ7AHxPGTwbGz2z9LZhY3
>YCQAn2In/XgrWPw6RTvk23W3F3msevg6
>=fNop
>-----END PGP SIGNATURE-----