Le 26 janv. 2012 à 18:38, jahanian a écrit :
>
> On Jan 26, 2012, at 1:45 AM, Jean-Daniel Dupas wrote:
>
>>
>> Le 25 janv. 2012 à 22:02, jahanian a écrit :
>>
>>>
>>> On Jan 25, 2012, at 10:41 AM, Jean-Daniel Dupas wrote:
>>>
>>>>
>>>> Le 25 janv. 2012 à 18:33, jahanian a écrit :
>>>>
>>>>>
>>>>> On Jan 24, 2012, at 4:39 PM, Jean-Daniel Dupas wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> 3-macros: In Obj-C and CoreFoundation, the recommended way to localize
>>>>>> string are respectively to use NSLocalizedString(key, comment) and
>>>>>> CFCopyLocalizedString(key, comment) macros family.
>>>>>> It is common to use these macros as format string, but as they expand to
>>>>>> method/function call, clang will warn about "non literal string" used as
>>>>>> format string.
>>>>>> So, this patch is a tentative to prevent diagnostic for this common
>>>>>> usage. It inhibits the "non literal string format" diagnostic when the
>>>>>> format type if NS/CFstring and the format argument is a macro expansion.
>>>>>> Note that while the CFCopyLocalizedString() expands to a function
>>>>>> properly tagged with the "format_arg" attribute, we can't rely on it,
>>>>>> because interpreting the 'key' parameter as a format string is incorrect
>>>>>> IMHO.
>>>>>> It is a common practice to use some kind of descriptive name for the key
>>>>>> (i.e. "UNEXPECTED_ERROR_TITLE") instead of the string value ("An
>>>>>> unexpected error occurred: %@").
>>>>>> Moreover, NSLocalizedString() does not use the "format_arg" attribute.
>>>>>>
>>>>>
>>>>> I don't seem to be getting the warning on the test case, and I don't
>>>>> think you have yet checked in the patch.
>>>>
>>>> You're right. The tested warning is not enabled by default, and I forgot
>>>> to add it to the test command line (or to enable it using pragma
>>>> diagnostic).
>>>> I attach an updated version of the remaining patch with this issue fixed.
>>>
>>> This patch is OK for the purpose you mentioned.
>>
>> Thanks for the reviews.
>>
>> This patch depends on the first one though, so I will apply it when the
>> first one (attached to this mail) is approved.
>>
>
> I see. Patch is OK. However, there may be concern about performance of
> passing a StringRef down. Can you instead pass an
> enum value of routine names you are interested in?
>
> - Fariborz
OK. I will have a look at it.
-- Jean-Daniel
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
