On Mon, Jan 30, 2012 at 11:26 AM, Jean-Daniel Dupas <[email protected]> wrote: > > Le 30 janv. 2012 à 18:42, Nico Weber a écrit : > >> On Mon, Jan 30, 2012 at 9:37 AM, Nico Weber <[email protected]> wrote: >>> On Mon, Jan 30, 2012 at 9:32 AM, Jean-Daniel Dupas >>> <[email protected]> wrote: >>>> >>>> Le 30 janv. 2012 à 17:37, Nico Weber a écrit : >>>> >>>>> Cool! >>>>> >>>>> One thing that this warned on in the chromium source was: >>>>> >>>>> ../../webkit/plugins/npapi/plugin_web_event_converter_mac.mm:213:37: >>>>> error: format specifies type 'wchar_t *' (aka 'wchar_t *') but the >>>>> argument has type 'const WebUChar *' (aka 'const unsigned short *') >>>>> [-Werror,-Wformat] >>>>> [NSString stringWithFormat:@"%S", key_event.text]); >>>>> ~^ ~~~~~~~~~~~~~~ >>>>> >>>>> It looks like the current %S warning checks if the parameter type is >>>>> wchar_t. >>>>> http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html#//apple_ref/doc/uid/TP40004265 >>>>> says that %S is always utf16 (or ucs16, it doesn't say). `man 3 >>>>> printf` says %S is for wchar_t, but I guess it refers to the wchar_t >>>>> type that was effective when libc / AppKit was built. On my OS X 10.6 >>>>> box, it looks like printf expects a four byte wchar_t, while AppKit >>>>> expects a two byte wchar_t. Since that code snippet above is about >>>>> AppKit, the code is right and the warning is wrong (chromium isn't >>>>> compiled with -fshort-wchar, but AppKit was, and the warning really >>>>> needs to check against the wchar_t type that was used to build the >>>>> library it's talking to). >>>>> >>>>> Since %S can expect utf32 (lib) or utf16 (AppKit), the %S warning is >>>>> currently fairly useless. %S seems to be an Apple extension, so should >>>>> we just hardcode %S to look for a 32bit type in c strings and for a >>>>> 16bit type in @"" strings? >>>>> Should we disable the warning for %S >>>>> altogether? >>>> >>>> >>>> I think we should just extends the printf checker to handle %S differently >>>> when the format spec is 'NSString' (it already has special handling for >>>> %@). >>> >>> The current %S warning implementation is wrong for just printf() as >>> well: It compares the argument to wchar_t, but if you build your >>> program with -fshort-wchar, clang won't warn (since your program uses >>> wchar_t) yet printf won't work (because libc was built without >>> -fshort-wchar). If you store your characters in an uint32* and pass >>> that to printf() and build your program with -fshort-wchar, clang will >>> warn (because uint32* doesn't match wchar_t* with -fshort-whar) yet >>> the program will work correctly. >> >> How about: We support it for format spec NSString and expect the type >> to be some 16bit type, and we always warn on %S else? (It can't be >> portable elsewhere, and as Hans said, the Ubuntu manpage even says "Do >> not use.") > > Yes, but it says to use %ls instead, which has the same semantic. Moreover, > %S is supported on BSD (and so on OS X). > > I think we should continue to treat %S and %ls as pointer to wchar_t (as > documented) and ignore compatibility issue introduce by -fshort-wchar. > The -fshort-wchar flag will almost always produce broken code anyway, as it > breaks all calls to functions that use a wchar_t arguments, unless the user > make sure he uses a lib c compiled with this flag too. > And when the library and the code both use -fshort-wchar, clang properly > handle '%S'.
Ok, I'll prepare a patch for your suggestion then (change %S to expect 16 bit types in nsstring types, don't change the 'normal' current behavior). Thanks! _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
