On Wed, Apr 13, 2011 at 2:07 PM, jahanian <[email protected]> wrote: > > On Apr 13, 2011, at 2:02 PM, Eli Friedman wrote: > >> On Wed, Apr 13, 2011 at 1:31 PM, Fariborz Jahanian <[email protected]> >> wrote: >>> Author: fjahanian >>> Date: Wed Apr 13 15:31:26 2011 >>> New Revision: 129465 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=129465&view=rev >>> Log: >>> No fixit hint for builtin expressions which are >>> defined in a macro. // rdar://9091893 >>> >>> Added: >>> cfe/trunk/test/FixIt/no-macro-fixit.c >>> Modified: >>> cfe/trunk/lib/Sema/SemaExpr.cpp >>> >>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=129465&r1=129464&r2=129465&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Apr 13 15:31:26 2011 >>> @@ -10070,9 +10070,6 @@ >>> return; >>> } >>> >>> - SourceLocation Open = E->getSourceRange().getBegin(); >>> - SourceLocation Close = >>> PP.getLocForEndOfToken(E->getSourceRange().getEnd()); >>> - >>> Diag(Loc, diagnostic) << E->getSourceRange(); >>> >>> if (IsOrAssign) >>> @@ -10082,9 +10079,14 @@ >>> Diag(Loc, diag::note_condition_assign_to_comparison) >>> << FixItHint::CreateReplacement(Loc, "=="); >>> >>> - Diag(Loc, diag::note_condition_assign_silence) >>> - << FixItHint::CreateInsertion(Open, "(") >>> - << FixItHint::CreateInsertion(Close, ")"); >>> + SourceLocation Open = E->getSourceRange().getBegin(); >>> + SourceLocation Close = E->getSourceRange().getEnd(); >>> + if (!Open.isMacroID() && !Close.isMacroID()) { >>> + SourceLocation LocForEndOfToken = PP.getLocForEndOfToken(Close); >>> + Diag(Loc, diag::note_condition_assign_silence) >>> + << FixItHint::CreateInsertion(Open, "(") >>> + << FixItHint::CreateInsertion(LocForEndOfToken, ")"); >>> + } >>> } >>> >>> /// \brief Redundant parentheses over an equality comparison can indicate >>> >>> Added: cfe/trunk/test/FixIt/no-macro-fixit.c >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/no-macro-fixit.c?rev=129465&view=auto >>> ============================================================================== >>> --- cfe/trunk/test/FixIt/no-macro-fixit.c (added) >>> +++ cfe/trunk/test/FixIt/no-macro-fixit.c Wed Apr 13 15:31:26 2011 >>> @@ -0,0 +1,15 @@ >>> +// RUN: %clang_cc1 -pedantic -fixit -x c %s >>> +// rdar://9091893 >>> + >>> +#define va_arg(ap, type) __builtin_va_arg(ap, type) >>> +typedef __builtin_va_list va_list; >>> + >>> +void myFunc() { >>> + va_list values; >>> + >>> + int value; >>> + >>> + while (value = va_arg(values, int)) { // expected-warning {{using the >>> result of an assignment as a condition without parentheses}} \ >>> + // expected-note {{use '==' to >>> turn this assignment into an equality comparison}} >>> + } >>> +} >> >> This doesn't really look like the right solution for a few reasons: >> 1. We don't really want to suppress the note, even if there isn't a fixit. > > We are only suppressing the 'fixit' note.
The user might not be aware the warning can be suppressed with parentheses, and this fix completely hides the note, not only the fixit. -Eli >> 2. The diagnostic machinery should be able to automatically suppress >> fixits that end up in macros. >> 3. Why can't we come up with the "right" source location here? > > I talked to Doug. And he said that we can't. I suggest he answers that. > > - fariborz > >> >> -Eli > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
