[cfe-commits] [patch] Added a suggestion when a std::string is passed to printf()

[Sam Panzer]

Thanks for the input!

To address Hal's questions:
1. No, I did not check if c_str() matches the expected type. I expect that
the correct approach would be to check all c_str() member functions and (if
none of them match) all conversion operators from their return types to the
expected type.

2. This patch did not handle wide-character strings, but adding support is
very simple. I expect to have a new patch implementing these two
suggestions shortly.

Matthieu,
I believe that %ls and %s inputs to printf() are treated as (w)char const*.
Is there an issue that I am unaware of, or should I have specified `char
const *` rather than `char *` in my original message?

Here is one other question: With this patch, incorrectly passing an
std::string to printf generates an error (can't pass non-POD member), a
warning (printf format doesn't match), and a note (the c_str() suggestion).
Is this the behavior we want?

On Sat, Jun 2, 2012 at 5:27 AM, Matthieu Monrocq <matthieu.monrocq at gmail.com
> wrote:

>
>
> On Sat, Jun 2, 2012 at 1:06 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>
>> Sam,
>>
>> Two quick questions:
>>
>> 1. Do you check the return type of c_str() to make sure it matches what
>> you expect? If you do check the type and it does not match, you should
>> also check if the returned type has a conversion operator to the type
>> needed.
>>
>> 2. Does this handle wide-character strings?
>>
>> I like the idea of adding this note, I think it will be helpful for
>> many users.
>>
>>  -Hal
>>
>> On Fri, 1 Jun 2012 14:59:42 -0700
>> Sam Panzer <panzer at google.com> wrote:
>>
>> > Hi,
>> >
>> > We had a request for more user-friendly printf() diagnostics, which
>> > asked for a note suggesting a call to .c_str() when passing a c++
>> > string to printf when a char* is expected, and I thought it would
>> > make a good first submission:
>> >
>> > Currently code such as
>> > >
>> > >  string foo;
>> > >
>> > >  printf("Oh dear %s", foo);
>> > >
>> > > produces a reasonable (if somewhat technical) diagnostic:
>> > >
>> > >  error: cannot pass object of non-trivial type 'string' (aka
>> > >> 'basic_string<char>') through variadic function; call will abort
>> > >> at runtime [-Wnon-pod-varargs]
>> > >
>> > >    printf("Oh dear %s", foo);
>> > >
>> > >
>> > >> That confuses some people (such as people who happened to be
>> > >> sitting near me today).  It would be easy for them to understand
>> > >> if this could special-case the situation where the call is to a
>> > >> *printf-like function and the non-POD in question is a string (or
>> > >> std::string), to suggest that the solution might be to add the
>> > >> missing ".c_str()".
>> > >
>> > >
>> > >> When format string checking is active for the function/argument in
>> > >> question it would also be possible to report on the expected
>> > >> argument type.
>> > >
>> > >
>> > >> (Of course it's also not ideal to report that the "call will abort
>> > >> at runtime" when generating an error rather than a warning, but
>> > >> that might not be worth fixing.)
>> > >
>> > >
>> > I attached a patch which emits this note when an object defining
>> > a .c_str() member function is passed instead of a char*, along with a
>> > test file. Searching for c_str() was intended to allow the note to
>> > fire for std::string replacements.
>> >
>> > -Sam
>>  <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>>
>
> One note:
>
> c_str() is supposed to return a `char const*` and the conversion from
> `char const*` to `char*` is deprecated.
>
> -- Matthieu
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120604/a7f05c18/attachment.html>