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