[cfe-commits] r129465 - in /cfe/trunk: lib/Sema/SemaExpr.cpp test/FixIt/no-macro-fixit.c

[Eli Friedman]

On Wed, Apr 13, 2011 at 1:31 PM, Fariborz Jahanian <fjahanian at apple.com> 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.
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?

-Eli