<div>Thanks for the input!</div><div><br></div><div>To address Hal's questions:</div><div>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.</div>
<div><br></div><div>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.</div><div><br></div><div>Matthieu, </div>
<div>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?</div><div><br>
</div><div>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?</div>
<br><div class="gmail_quote">On Sat, Jun 2, 2012 at 5:27 AM, Matthieu Monrocq <span dir="ltr"><<a href="mailto:matthieu.monrocq@gmail.com" target="_blank">matthieu.monrocq@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><br><div class="gmail_quote"><div><div class="h5">On Sat, Jun 2, 2012 at 1:06 AM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Sam,<br>
<br>
Two quick questions:<br>
<br>
1. Do you check the return type of c_str() to make sure it matches what<br>
you expect? If you do check the type and it does not match, you should<br>
also check if the returned type has a conversion operator to the type<br>
needed.<br>
<br>
2. Does this handle wide-character strings?<br>
<br>
I like the idea of adding this note, I think it will be helpful for<br>
many users.<br>
<br>
-Hal<br>
<div><div><br>
On Fri, 1 Jun 2012 14:59:42 -0700<br>
Sam Panzer <<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>> wrote:<br>
<br>
> Hi,<br>
><br>
> We had a request for more user-friendly printf() diagnostics, which<br>
> asked for a note suggesting a call to .c_str() when passing a c++<br>
> string to printf when a char* is expected, and I thought it would<br>
> make a good first submission:<br>
><br>
> Currently code such as<br>
> ><br>
> > string foo;<br>
> ><br>
> > printf("Oh dear %s", foo);<br>
> ><br>
> > produces a reasonable (if somewhat technical) diagnostic:<br>
> ><br>
> > error: cannot pass object of non-trivial type 'string' (aka<br>
> >> 'basic_string<char>') through variadic function; call will abort<br>
> >> at runtime [-Wnon-pod-varargs]<br>
> ><br>
> > printf("Oh dear %s", foo);<br>
> ><br>
> ><br>
> >> That confuses some people (such as people who happened to be<br>
> >> sitting near me today). It would be easy for them to understand<br>
> >> if this could special-case the situation where the call is to a<br>
> >> *printf-like function and the non-POD in question is a string (or<br>
> >> std::string), to suggest that the solution might be to add the<br>
> >> missing ".c_str()".<br>
> ><br>
> ><br>
> >> When format string checking is active for the function/argument in<br>
> >> question it would also be possible to report on the expected<br>
> >> argument type.<br>
> ><br>
> ><br>
> >> (Of course it's also not ideal to report that the "call will abort<br>
> >> at runtime" when generating an error rather than a warning, but<br>
> >> that might not be worth fixing.)<br>
> ><br>
> ><br>
> I attached a patch which emits this note when an object defining<br>
> a .c_str() member function is passed instead of a char*, along with a<br>
> test file. Searching for c_str() was intended to allow the note to<br>
> fire for std::string replacements.<br>
><br>
> -Sam<br>
<span><font color="#888888"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank"></a></font></span></div></div></blockquote></div></div><div> <br>One note:<br></div><div><br>c_str() is supposed to return a `char const*` and the conversion from `char const*` to `char*` is deprecated.<span class="HOEnZb"><font color="#888888"><br>
<br>-- Matthieu <br></font></span></div></div><br>
</blockquote></div><br>