On Apr 13, 2011, at 2:15 PM, Eli Friedman wrote: > 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.
OK. I can add another note to that effect. But I wait for Doug's feedback first. - fj > > -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
