On 1/6/19 4:34 PM, Martin Sebor wrote:
> Attached is an updated patch with the wording change to the manual
> discussed below and rebased on the top of today's trunk.
>
> Martin
>
> PS Thanks for the additional info, Iain.
>
> On 1/5/19 10:53 AM, Iain Sandoe wrote:
>>
>>> On 5 Jan 2019, at 17:39, Martin Sebor <mse...@gmail.com> wrote:
>>>
>>> On 1/5/19 3:31 AM, Iain Sandoe wrote:
>>>> Hi Martin,
>>>>> On 4 Jan 2019, at 22:30, Mike Stump <mikest...@comcast.net> wrote:
>>>>>
>>>>> On Jan 4, 2019, at 2:03 PM, Martin Sebor <mse...@gmail.com> wrote:
>>>>>>
>>>>>> The improved handling of attribute positional arguments added
>>>>>> in r266195 introduced a regression on Darwin where attribute
>>>>>> format with the CFString archetype accepts CFString* parameter
>>>>>> types in positions where only char* would otherwise be allowed.
>>>>>
>>>>> You didn't ask Ok? I'll assume you want a review... The darwin bits and
>>>>> the testsuite bits look fine.
>>>>>
>>>>>>
>>>>>> Index: gcc/doc/extend.texi
>>>>>> ===================================================================
>>>>>> --- gcc/doc/extend.texi (revision 267580)
>>>>>> +++ gcc/doc/extend.texi (working copy)
>>>>>> @@ -22368,10 +22368,12 @@ bit-fields. See the Solaris man page for
>>>>>> @code{cm
>>>>>>
>>>>>> @node Darwin Format Checks
>>>>>> @subsection Darwin Format Checks
>>>>>> -Darwin targets support the @code{CFString} (or @code{__CFString__})
>>>>>> in the format
>>>>>>
>>>>>> -attribute context. Declarations made with such attribution are parsed
>>>>>> for correct syntax
>>>>>>
>>>>>> -and format argument types. However, parsing of the format string
>>>>>> itself is currently undefined
>>>>>>
>>>>>> -and is not carried out by this version of the compiler.
>>>>>> +In addition to the full set of archetypes, Darwin targets also support
>>>>>>
>>>>>> +the @code{CFString} (or @code{__CFString__}) archetype in the
>>>>>> @code{format}
>>>>>>
>>>>>> +attribute. Declarations with this archetype are parsed for correct
>>>>>> syntax
>>>>>>
>>>>>> +and argument types. However, parsing of the format string itself and
>>>>>>
>>>>>> +validating arguments against it in calls to such functions is currently
>>>>>>
>>>>>> +not performed.
>>>>>> Additionally, @code{CFStringRefs} (defined by the
>>>>>> @code{CoreFoundation} headers) may
>>>>>>
>>>>>> also be used as format arguments. Note that the relevant headers are
>>>>>> only likely to be
>>>>>>
>>>>>>
>>>> I find “archetype(s)” to be an usual (and possibly unfamiliar to many)
>>>> word for this context.
>>>>
>>>> how about:
>>>> s/archetype in/variant for the/
>>>> and then
>>>> s/with this archetype/with this variant/
>>>> in the following sentence.
>>>> However, just 0.02GBP .. (fixing the fails is more important than
>>>> bikeshedding the wording).
>>>>
>>>
>>> Thanks for chiming in! I used archetype because that's the term
>>> used in the attribute format specification to describe the first
>>> argument. I do tend to agree that archetype alone may not be
>>> sufficiently familiar to all users. I'm happy to add text to
>>> make that clear. Would you find the following better?
>>>
>>> In addition to the full set of format archetypes (attribute
>>> format style arguments such as @code{printf}, @code{scanf},
>>> @code{strftime}, and @code{strfmon}), Darwin targets also
>>> support the @code{CFString} (or @code{__CFString__}) archetype…
>>
>> Yes, that makes is clearer
>>
>> (as an aside, I think that to many people the meaning of archetype - as
>> ‘generic’ or ‘root example’
>>
>> etc tends to come to mind before the ‘template/mold’ meaning … however
>> couldn’t think of
>>
>> a better term that’s not already overloaded).
>>
>>> FWIW, I wanted to figure out how the CFString attribute made it
>>> possible to differentiate between printf and scanf (and the other)
>>> kinds of functions, for example, so I could add new tests for it,
>>> but I couldn't tell that from the manual. So I'm trying to update
>>> the text to make it clear that although CFString is just like
>>> the sprintf and scanf format arguments/archetypes, beyond
>>> validating declarations that use it, the attribute serves no
>>> functional purpose, so the printf/scanf distinction is moot.
>>
>> The CFString container** is more general than our implementation, e.g. it
>> should be able
>>
>> to contain a variety of string formats (e.g. UTF etc.). AFAIR at the time
>> I implemented
>>
>> it for FSF GCC (it was originally in the Apple Local branch), we didn’t have
>> sufficient parsing
>>
>> support for such things (and the support in the Apple-local branch didn’t
>> look applicable).
>>
>>
>> If we do more sophisitcated checks, we probably need to take cognisance of
>> the fact that a
>>
>> fully-implemented CFString impl can have non-ascii payload. I suspect
>> (although honestly
>>
>> it’s been a while, I maybe mis-remember) that was the reason I didn’t try to
>> implement the
>>
>> inspection at the time (if so, there’s probably a code comment to that
>> effect).
>>
>>
>>> Out of curiosity, is the attribute used for function call
>>> validation by compilers other than GCC? I couldn't find
>>> anything online.
>>
>> It used to be used for the platform GCC, when that was the “system compiler"
>> (last edition
>>
>> apple 4.2.1) and I therefore assume it’s used by clang - but actually
>> haven’t double-checked
>>
>> at the time we added it - it was relevant.
>>
>> Let’s revisit this in 10, and see if we can’t sync up with the platform
>> expectations from the clang impl.
>>
>>
>> thanks for taking care of this,
>>
>> Iain
>>
>> ** it’s actually a simple ObjectiveC object that happens to be supported by
>> the CoreFoundation (C)
>>
>> classes as well as ObjectiveC .. making it possible to pass around general
>> string containers between
>>
>> the languages.
>>
>
>
> gcc-88638.diff
>
> PR target/88638 - FAIL: fsf-nsstring-format-1.s on darwin
>
> gcc/c-family/ChangeLog:
>
> PR target/88638
> * c-attribs.c (positional_argument): Call valid_format_string_type_p
> and issue errors if it fails.
> * c-common.h (valid_format_string_type_p): Declare.
> * c-format.c (valid_stringptr_type_p): Rename...
> (valid_format_string_type_p): ...to this and make extern.
> (handle_format_arg_attribute): Adjust to new name.
> (check_format_string): Same.
>
> gcc/testsuite/ChangeLog:
>
> PR target/88638
> * gcc.dg/format/attr-8.c: New test.
> * gcc.dg/darwin-cfstring-format-1.c: Adjust diagnostics.
> * gcc.dg/format/attr-3.c: Same.
> * obj-c++.dg/fsf-nsstring-format-1.mm: Same.
> * objc.dg/fsf-nsstring-format-1.m: Same.
>
> gcc/ChangeLog:
>
> PR target/88638
> * doc/extend.texi (Darwin Format Checks): Clarify.
OK
jeff
