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. > 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
