[cfe-commits] r149268 - in /cfe/trunk: lib/Sema/SemaChecking.cpp test/SemaObjC/format-strings-objc.m

[Jean-Daniel Dupas]

Le 30 janv. 2012 à 22:10, Eli Friedman a écrit :

> On Mon, Jan 30, 2012 at 11:46 AM, Jean-Daniel Dupas
> <devlists at shadowlab.org> wrote:
>> Author: jddupas
>> Date: Mon Jan 30 13:46:17 2012
>> New Revision: 149268
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=149268&view=rev
>> Log:
>> Disable "non literal format string" for NSString that result from a macro expansion.
>> This is to prevent diagnostic when using NSLocalizedString or CFCopyLocalizedString
>> macros which are usually used in place of NS and CF strings literals.
>> 
>> 
>> Modified:
>>    cfe/trunk/lib/Sema/SemaChecking.cpp
>>    cfe/trunk/test/SemaObjC/format-strings-objc.m
>> 
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=149268&r1=149267&r2=149268&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jan 30 13:46:17 2012
>> @@ -1586,6 +1586,13 @@
>>                              format_idx, firstDataArg, Type))
>>     return;  // Literal format string found, check done!
>> 
>> +  // Do not emit diag when the string param is a macro expansion and the
>> +  // format is either NSString or CFString. This is a hack to prevent
>> +  // diag when using the NSLocalizedString and CFCopyLocalizedString macros
>> +  // which are usually used in place of NS and CF string literals.
>> +  if (Type == FST_NSString && Args[format_idx]->getLocStart().isMacroID())
>> +    return;
>> +
>>   // If there are no arguments specified, warn with -Wformat-security, otherwise
>>   // warn only with -Wformat-nonliteral.
>>   if (NumArgs == format_idx+1)
>> 
>> Modified: cfe/trunk/test/SemaObjC/format-strings-objc.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-strings-objc.m?rev=149268&r1=149267&r2=149268&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaObjC/format-strings-objc.m (original)
>> +++ cfe/trunk/test/SemaObjC/format-strings-objc.m Mon Jan 30 13:46:17 2012
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -fsyntax-only -verify %s
>> +// RUN: %clang_cc1 -Wformat-nonliteral -fsyntax-only -verify %s
>> 
>>  //===----------------------------------------------------------------------===//
>>  // The following code is reduced using delta-debugging from
>> @@ -107,3 +107,11 @@
>>   NSString * ns3 = ns1;
>>   NSLog(ns3); // expected-warning {{format string is not a string literal}}}
>>  }
>> +
>> +// Do not emit warnings when using NSLocalizedString
>> +extern NSString *GetLocalizedString(NSString *str);
>> +#define NSLocalizedString(key) GetLocalizedString(key)
>> +
>> +void check_NSLocalizedString() {
>> +  [Foo fooWithFormat:NSLocalizedString(@"format"), @"arg"]; // no-warning
>> +}
> 
> I'm not sure this is the right approach... see also
> http://llvm.org/bugs/show_bug.cgi?id=3814 , which is essentially the
> same issue.
> 

gettext convention are not the same as Cocoa ones. 

From gettext man page:  "By convention, [msgid] is the English version of the message, with non-ASCII characters replaced by ASCII approximations." which means that the gettext macro argument should be a valid format string when used as printf argument.
If so, the gettext functions declaration should use the format_arg() attribute which is already handled in clang.

Indeed, using a recent version of gettext (which use format_arg), I get the correct behavior:

---- fmt.c ---
#include <stdio.h>
#include <libintl.h>
#include <gettext-po.h>

int test(int arg) {
	return printf(gettext("this is a format string: %s"), arg);
}
---------------

fmt.c:7:51: warning: format specifies type 'char *' but the argument has type 'int'
        return printf(gettext("this is a format string: %s"), arg);
                                                        ~^    ~~~

We may have to improve the support of multiple format_arg attributes, but I'm not sure what the semantic should be in such case.

The problem with Cocoa localization macro is that there is not such convention, and it is very common to use a key that is not a version of the localized string at all. So even if they were declared using format_arg (which is not the case), it would not help.


-- Jean-Daniel




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120131/3b917ca4/attachment.html>