Some minor comments, and I'll let others start chiming in here to make sure there is general agreement about the approach:<div><br></div><div><br></div><div><div>+def warn_non_pod_vararg_with_format_string : Warning<</div>
<div>+  "cannot pass non-POD object of type %0 to variadic function; "</div><div>+  "expected type was %1">,</div></div><div><br></div><div>I'd like to mention that the expectation stems from a format string here:</div>
<div><br></div><div>'... variadic function; the format string expects it to have type %1'</div><div><br></div><div>or some such...</div><div><br></div><div><div>+  // Used for determining in which context a type is considered POD</div>
<div>+  enum PODType {</div><div>+    POD,</div><div>+    CXX11_POD,</div><div>+    ObjC_Lifetime_POD,</div><div>+    NON_POD</div><div>+  };</div><div><br></div><div>The LLVM coding conventions on enums are much more precise than other things. I would vaguely expect it to look like:</div>
<div><br></div><div>enum PODKind {</div><div>  PK_NonPOD,</div><div>  PK_POD,</div><div>  PK_CXX11POD,</div><div>  PK_ObjCLifetimePOD</div><div>};</div><div><br></div><div>+</div><div>+  // Determines which PODType fits an expression.</div>
<div>+  PODType levelOfTriviality(const Expr *E);</div><div>+</div><div><br></div><div>Actually, looking at how this function is implemented, I think you're going about it the wrong way. This isn't a generalizable concept, it is a very specific and precise query: isValidTypeForVarargArgument or some such. It should return true or false, no enum required.</div>
<div><br></div><div><div>+                                              bool HasformatString = false);</div></div><div><br></div><div>s/HasformatString/HasFormatString/</div><div><br></div><div><div>+// Given a variadic function, decides whether it looks like we have a</div>
<div>+// printf-style format string and arguments)</div><div>+bool Sema::hasFormatStringArgument(FunctionDecl *FDecl,</div><div>+                                   Expr **Args, unsigned NumArgs) {</div></div><div><br></div>
<div>I don't see you using this function from anywhere but new code -- is there a reason that SemaChecking can't use this logic?</div><div><br></div><div><div>+    // Decide if we're probably inspecting a printf-like function</div>
<div>+</div><div>+    bool HasFormatString = hasFormatStringArgument(FDecl, Args, NumArgs);</div></div><div><br></div><div>I think the comment is now extraneous.</div><div><br></div><br><div class="gmail_quote">On Wed, Jun 6, 2012 at 4:56 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">Parens removed, though operator-> is unavailable here. For future reference, is Clang averse to extra variable declarations in favor of extra method calls? </blockquote>
</div><br></div>