And here is another update that replaces the default non-POD error with a smarter (printf-specific) one, rather than just tacking on an extra warning.<div><br></div><div>-Sam<br><br><div class="gmail_quote">On Mon, Jun 4, 2012 at 6:17 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It turns out that intercepting the non-POD error is quite a bit more complicated than getting the basic functionality of this patch to work, so I decided to work on that as a separate patch.<div>
<br></div><div>Here is the newer patch. The important features include:</div>
<div> - When a non-POD argument is passed to a printf-like function, it is checked for a c_str() member function that takes no parameters and returns a type that makes printf happy, the extra note is emitted with a FixItHint.</div>
<div> - This works for standard and wide-char strings (and anything else that people have thought up), since it lets analyze_printf::ArgTypeResult figure out the type matching.</div><div> - I don't check for conversion operators, since we (usually) get a char* out of c_str().</div>
<div> - Sema gained a function to find all C++ members of a given kind, a generalization of what I used previously.</div><div><br></div><div>What I'm missing for Chandler's request of combining the error and warning is:</div>
<div> - Code that detects that it is looking at a printf-like varargs function. Something similar appears in lib/Sema/SemaChecking.cpp: it looks like iterating over specific_attr_iterator<FormatAttr> as happens in Sema::CheckFunctionCall is the best option. Is this correct?</div>
<div><br></div><div>Thanks again!<div><div class="h5"><br><br><div class="gmail_quote">On Mon, Jun 4, 2012 at 2:10 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div>On Mon, Jun 4, 2012 at 2:09 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>Now I'm wondering how much of this behavior should be "string"-specific. If we're paying attention to conversion operators from non-POD types, why not check for all possible conversion operators? I think this is a better solution than checking to see if there's a conversion operator from the return type of a c_str() function.</div>
</blockquote><div><br></div></div><div>I think conversion operators aren't really as interesting and at least should be handled as a separate follow-on patch. I also think they're completely uninteresting when considering the result of 'c_str()'. All of the interesting cases actually return the character pointer we're looking for.</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>Not depending on std::string was the plan, and in fact how I implemented it :)</div><div><div><div><div><br><div class="gmail_quote">On Mon, Jun 4, 2012 at 1:44 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="gmail_quote">On Mon, Jun 4, 2012 at 1:37 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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?</blockquote>
</div><br></div><div>I don't think so... Ideally:</div><div><br></div><div>1) If we are about to issue a an error (non-POD object passed through printf), check whether the non-POD object has a 'c_str()' method that returns a type which matches the format specifier. If so, use a specific diagnostic message for the error, attach a fixit-hint suggesting '.c_str()', and continue parsing as-if the user had done that.</div>
<div><br></div><div>2) If there is no error (passing a std::string* perhaps), then in the warning message, give a more precise message than 'cannot convert std::string* to const char*' or whatever by checking if there is a c_str() method that matches the type of the printf.</div>
<div><br></div><div><br></div><div>One thing I would encourage you to do: don't base this on 'std::string'. I actually think the error/warning should fire for any class type with a c_str method that returns a viable type. That way non-standard string libraries will get the same benefit if they conform the the same conceptual interface as the standard string library.</div>
</blockquote></div><br></div></div>
</div></div></blockquote></div></div><br>
</blockquote></div><br>
</div></div></div>
</blockquote></div><br></div>