[PATCH] restore CFString handling in attribute format (PR 88638)

Iain Sandoe iain@sandoe.co.uk
Mon Jan 7 09:26:00 GMT 2019


> On 6 Jan 2019, at 23:34, Martin Sebor <msebor@gmail.com> wrote:
> 
> Attached is an updated patch with the wording change to the manual
> discussed below and rebased on the top of today's trunk.

Works for me as well.
thanks for the patch.

Iain

> 
> 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 <msebor@gmail.com> wrote:
>>> 
>>> On 1/5/19 3:31 AM, Iain Sandoe wrote:
>>>> Hi Martin,
>>>>> On 4 Jan 2019, at 22:30, Mike Stump <mikestump@comcast.net> wrote:
>>>>> 
>>>>> On Jan 4, 2019, at 2:03 PM, Martin Sebor <msebor@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>



More information about the Gcc-patches mailing list