<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">OK. I will do that.<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Mar 11, 2016, at 4:15 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Fri, Mar 11, 2016 at 4:14 PM, Bob Wilson via cfe-commits <span dir="ltr" class=""><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="">I’m not sure how to interpret that. It says: "Clang must recover from errors as if the fix-it had been applied.” I suppose that format-security could be an error if you’re building with -Werror, but I had been interpreting that to mean an error that would block further compilation. Can someone clarify this expectation?<div class=""><br class=""></div><div class="">I don’t think we can change -Wformat-security to be a note.</div></div></blockquote><div class=""><br class=""></div><div class="">The suggestion isn't to change it to be a note, but to keep the warning but move the fixit to a note associated with the warning.<br class=""><br class="">That way -fix-it doesn't automatically apply the fix-it, which avoids the issue of recovery as-if the fixit was applied.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""> It is an existing warning, and people expect us to match GCC on it as well.<div class=""><div class="h5"><br class=""><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Mar 11, 2016, at 4:03 PM, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank" class="">thakis@chromium.org</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class="">I think <a href="http://clang.llvm.org/docs/InternalsManual.html#fix-it-hints" target="_blank" class="">http://clang.llvm.org/docs/InternalsManual.html#fix-it-hints</a> says that if a fixit is on a warning, then clang should process the code as if the fixit had been applied. That's not the case here, so I think the fixit should be on a note instead.<br class=""></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Fri, Mar 11, 2016 at 4:55 PM, Bob Wilson via cfe-commits <span dir="ltr" class=""><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: bwilson<br class="">
Date: Fri Mar 11 15:55:37 2016<br class="">
New Revision: 263299<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=263299&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=263299&view=rev</a><br class="">
Log:<br class="">
Add fix-it for format-security warnings.<br class="">
<br class="">
Added:<br class="">
    cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m<br class="">
Modified:<br class="">
    cfe/trunk/lib/Sema/SemaChecking.cpp<br class="">
    cfe/trunk/test/Sema/format-strings-fixit.c<br class="">
<br class="">
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=263299&r1=263298&r2=263299&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=263299&r1=263298&r2=263299&view=diff</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br class="">
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Mar 11 15:55:37 2016<br class="">
@@ -3621,20 +3621,32 @@ bool Sema::CheckFormatArguments(ArrayRef<br class="">
   // format is either NSString or CFString. This is a hack to prevent<br class="">
   // diag when using the NSLocalizedString and CFCopyLocalizedString macros<br class="">
   // which are usually used in place of NS and CF string literals.<br class="">
-  if (Type == FST_NSString &&<br class="">
-      SourceMgr.isInSystemMacro(Args[format_idx]->getLocStart()))<br class="">
+  SourceLocation FormatLoc = Args[format_idx]->getLocStart();<br class="">
+  if (Type == FST_NSString && SourceMgr.isInSystemMacro(FormatLoc))<br class="">
     return false;<br class="">
<br class="">
   // If there are no arguments specified, warn with -Wformat-security, otherwise<br class="">
   // warn only with -Wformat-nonliteral.<br class="">
-  if (Args.size() == firstDataArg)<br class="">
-    Diag(Args[format_idx]->getLocStart(),<br class="">
-         diag::warn_format_nonliteral_noargs)<br class="">
+  if (Args.size() == firstDataArg) {<br class="">
+    const SemaDiagnosticBuilder &D =<br class="">
+      Diag(FormatLoc, diag::warn_format_nonliteral_noargs);<br class="">
+    switch (Type) {<br class="">
+    default:<br class="">
+      D << OrigFormatExpr->getSourceRange();<br class="">
+      break;<br class="">
+    case FST_Kprintf:<br class="">
+    case FST_FreeBSDKPrintf:<br class="">
+    case FST_Printf:<br class="">
+      D << FixItHint::CreateInsertion(FormatLoc, "\"%s\", ");<br class="">
+      break;<br class="">
+    case FST_NSString:<br class="">
+      D << FixItHint::CreateInsertion(FormatLoc, "@\"%@\", ");<br class="">
+      break;<br class="">
+    }<br class="">
+  } else {<br class="">
+    Diag(FormatLoc, diag::warn_format_nonliteral)<br class="">
       << OrigFormatExpr->getSourceRange();<br class="">
-  else<br class="">
-    Diag(Args[format_idx]->getLocStart(),<br class="">
-         diag::warn_format_nonliteral)<br class="">
-           << OrigFormatExpr->getSourceRange();<br class="">
+  }<br class="">
   return false;<br class="">
 }<br class="">
<br class="">
<br class="">
Modified: cfe/trunk/test/Sema/format-strings-fixit.c<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-fixit.c?rev=263299&r1=263298&r2=263299&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-fixit.c?rev=263299&r1=263298&r2=263299&view=diff</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/test/Sema/format-strings-fixit.c (original)<br class="">
+++ cfe/trunk/test/Sema/format-strings-fixit.c Fri Mar 11 15:55:37 2016<br class="">
@@ -16,6 +16,8 @@ typedef __UINTMAX_TYPE__ uintmax_t;<br class="">
 typedef __PTRDIFF_TYPE__ ptrdiff_t;<br class="">
 typedef __WCHAR_TYPE__ wchar_t;<br class="">
<br class="">
+extern const char *NonliteralString;<br class="">
+<br class="">
 void test() {<br class="">
   // Basic types<br class="">
   printf("%s", (int) 123);<br class="">
@@ -94,6 +96,9 @@ void test() {<br class="">
   printf("%G", (long double) 42);<br class="">
   printf("%a", (long double) 42);<br class="">
   printf("%A", (long double) 42);<br class="">
+<br class="">
+  // nonliteral format<br class="">
+  printf(NonliteralString);<br class="">
 }<br class="">
<br class="">
 int scanf(char const *, ...);<br class="">
@@ -218,6 +223,7 @@ void test2(int intSAParm[static 2]) {<br class="">
 // CHECK: printf("%LG", (long double) 42);<br class="">
 // CHECK: printf("%La", (long double) 42);<br class="">
 // CHECK: printf("%LA", (long double) 42);<br class="">
+// CHECK: printf("%s", NonliteralString);<br class="">
<br class="">
 // CHECK: scanf("%99s", str);<br class="">
 // CHECK: scanf("%s", vstr);<br class="">
<br class="">
Added: cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m?rev=263299&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m?rev=263299&view=auto</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m (added)<br class="">
+++ cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m Fri Mar 11 15:55:37 2016<br class="">
@@ -0,0 +1,31 @@<br class="">
+// RUN: cp %s %t<br class="">
+// RUN: %clang_cc1 -x objective-c -triple x86_64-apple-darwin -Wno-objc-root-class -pedantic -Wall -fixit %t<br class="">
+// RUN: %clang_cc1 -x objective-c -triple x86_64-apple-darwin -Wno-objc-root-class -fsyntax-only -pedantic -Wall -Werror %t<br class="">
+// RUN: %clang_cc1 -x objective-c -triple x86_64-apple-darwin -Wno-objc-root-class -E -o - %t | FileCheck %s<br class="">
+<br class="">
+typedef signed char BOOL;<br class="">
+typedef unsigned int NSUInteger;<br class="">
+typedef struct _NSZone NSZone;<br class="">
+@class NSCoder, NSString, NSEnumerator;<br class="">
+@protocol NSObject  - (BOOL)isEqual:(id)object; @end<br class="">
+@protocol NSCopying  - (id)copyWithZone:(NSZone *)zone; @end<br class="">
+@protocol NSMutableCopying  - (id)mutableCopyWithZone:(NSZone *)zone; @end<br class="">
+@protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end<br class="">
+@interface NSObject <NSObject> {} @end<br class="">
+@interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding>  - (NSUInteger)length; @end<br class="">
+extern void NSLog(NSString *format, ...);<br class="">
+<br class="">
+/* This is a test of the various code modification hints that are<br class="">
+   provided as part of warning or extension diagnostics. All of the<br class="">
+   warnings will be fixed by -fixit, and the resulting file should<br class="">
+   compile cleanly with -Werror -pedantic. */<br class="">
+<br class="">
+extern NSString *NonliteralString;<br class="">
+<br class="">
+void test() {<br class="">
+  // nonliteral format<br class="">
+  NSLog(NonliteralString);<br class="">
+}<br class="">
+<br class="">
+// Validate the fixes.<br class="">
+// CHECK: NSLog(@"%@", NonliteralString);<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
cfe-commits mailing list<br class="">
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class="">
</blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></div></div></div></div><br class="">_______________________________________________<br class="">
cfe-commits mailing list<br class="">
<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class="">
<br class=""></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></div></body></html>