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