[cfe-commits] r135509 - in /cfe/trunk: include/clang/Basic/Diagnostic.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Overload.h lib/Sema/SemaOverload.cpp test/FixIt/fixit-function-call.cpp
Nick Lewycky
nlewycky at google.com
Tue Jul 19 13:28:11 PDT 2011
Thanks!
I tried testing this patch out since it fixes one of our bugs too, but found
something odd. Here's our testcase:
void Func(int *) { }
void test() {
int x;
Func(x);
}
"clang -fsyntax-only" on that doesn't emit the fix-it, *but* if you run
"clang -cc1 -fdiagnostics-parseable-fixits -x c++" then it does print:
fix-it:"b4070551.cc":{4:9-4:9}:"&"
I don't understand why it shows up with -fdiagnostics-parseable-fixits but
not as a fixit with just -fsyntax-only? Those two really shouldn't be
different. Could you please take a look?
Nick
On 19 July 2011 12:49, Anna Zaks <ganna at apple.com> wrote:
> Author: zaks
> Date: Tue Jul 19 14:49:12 2011
> New Revision: 135509
>
> URL: http://llvm.org/viewvc/llvm-project?rev=135509&view=rev
> Log:
> Add FixItHints in case a C++ function call is missing * or & operators on
> one/several of it's parameters (addresses http://llvm.org/PR5941).
>
> Added:
> cfe/trunk/test/FixIt/fixit-function-call.cpp
> Modified:
> cfe/trunk/include/clang/Basic/Diagnostic.h
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Overload.h
> cfe/trunk/lib/Sema/SemaOverload.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=135509&r1=135508&r2=135509&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
> +++ cfe/trunk/include/clang/Basic/Diagnostic.h Tue Jul 19 14:49:12 2011
> @@ -615,7 +615,7 @@
> /// only support 10 ranges, could easily be extended if needed.
> CharSourceRange DiagRanges[10];
>
> - enum { MaxFixItHints = 3 };
> + enum { MaxFixItHints = 6 };
>
> /// FixItHints - If valid, provides a hint with some code
> /// to insert, remove, or modify at a particular position.
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=135509&r1=135508&r2=135509&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jul 19
> 14:49:12 2011
> @@ -1592,7 +1592,9 @@
> "function (the implicit move assignment operator)|"
> "constructor (inherited)}0%1"
> " not viable: no known conversion from %2 to %3 for "
> - "%select{%ordinal5 argument|object argument}4">;
> + "%select{%ordinal5 argument|object argument}4; "
> + "%select{|dereference the argument with *|"
> + "take the address of the argument with &}6">;
> def note_ovl_candidate_bad_addrspace : Note<"candidate "
> "%select{function|function|constructor|"
> "function |function |constructor |"
>
> Modified: cfe/trunk/include/clang/Sema/Overload.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Overload.h?rev=135509&r1=135508&r2=135509&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Overload.h (original)
> +++ cfe/trunk/include/clang/Sema/Overload.h Tue Jul 19 14:49:12 2011
> @@ -528,6 +528,12 @@
> ovl_fail_final_conversion_not_exact
> };
>
> + enum OverloadFixItKind {
> + OFIK_Undefined = 0,
> + OFIK_Dereference,
> + OFIK_TakeAddress
> + };
> +
> /// OverloadCandidate - A single candidate in an overload set (C++ 13.3).
> struct OverloadCandidate {
> /// Function - The actual function that this candidate
> @@ -556,6 +562,21 @@
> /// function arguments to the function parameters.
> llvm::SmallVector<ImplicitConversionSequence, 4> Conversions;
>
> + /// The FixIt hints which can be used to fix the Bad candidate.
> + struct FixInfo {
> + /// The list of Hints (all have to be applied).
> + llvm::SmallVector<FixItHint, 4> Hints;
> +
> + /// The number of Conversions fixed. This can be different from the
> size
> + /// of the Hints vector since we allow multiple FixIts per
> conversion.
> + unsigned NumConversionsFixed;
> +
> + /// The type of fix applied.
> + OverloadFixItKind Kind;
> +
> + FixInfo(): NumConversionsFixed(0), Kind(OFIK_Undefined) {}
> + } Fix;
> +
> /// Viable - True to indicate that this overload candidate is viable.
> bool Viable;
>
>
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=135509&r1=135508&r2=135509&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue Jul 19 14:49:12 2011
> @@ -6732,6 +6732,85 @@
>
> namespace {
>
> +/// Try to find a fix for the bad conversion. Populate the ConvFix
> structure
> +/// on success. Produces the hints for the following cases:
> +/// - The user forgot to apply * or & operator to one or more arguments.
> +static bool TryToFixBadConversion(Sema &S,
> + const ImplicitConversionSequence &Conv,
> + OverloadCandidate::FixInfo &ConvFix) {
> + assert(Conv.isBad() && "Only try to fix a bad conversion.");
> +
> + const Expr *Arg = Conv.Bad.FromExpr;
> + if (!Arg)
> + return false;
> +
> + // The conversion is from argument type to parameter type.
> + const CanQualType FromQTy = S.Context.getCanonicalType(Conv.Bad
> + .getFromType());
> + const CanQualType ToQTy =
> S.Context.getCanonicalType(Conv.Bad.getToType());
> + const SourceLocation Begin = Arg->getSourceRange().getBegin();
> + const SourceLocation End =
> S.PP.getLocForEndOfToken(Arg->getSourceRange()
> + .getEnd());
> + bool NeedParen = true;
> + if (isa<ParenExpr>(Arg) || isa<DeclRefExpr>(Arg))
> + NeedParen = false;
> +
> + // Check if the argument needs to be dereferenced
> + // (type * -> type) or (type * -> type &).
> + if (const PointerType *FromPtrTy = dyn_cast<PointerType>(FromQTy)) {
> + // Try to construct an implicit conversion from argument type to the
> + // parameter type.
> + OpaqueValueExpr TmpExpr(Arg->getExprLoc(),
> FromPtrTy->getPointeeType(),
> + VK_LValue);
> + ImplicitConversionSequence ICS =
> + TryCopyInitialization(S, &TmpExpr, ToQTy, true, true, false);
> +
> + if (!ICS.isBad()) {
> + // Do not suggest dereferencing a Null pointer.
> + if (Arg->IgnoreParenCasts()->
> + isNullPointerConstant(S.Context,
> Expr::NPC_ValueDependentIsNotNull))
> + return false;
> +
> + if (NeedParen) {
> + ConvFix.Hints.push_back(FixItHint::CreateInsertion(Begin, "*("));
> + ConvFix.Hints.push_back(FixItHint::CreateInsertion(End, ")"));
> + } else {
> + ConvFix.Hints.push_back(FixItHint::CreateInsertion(Begin, "*"));
> + }
> + ConvFix.NumConversionsFixed++;
> + if (ConvFix.NumConversionsFixed == 1)
> + ConvFix.Kind = OFIK_Dereference;
> + return true;
> + }
> + }
> +
> + // Check if the pointer to the argument needs to be passed
> + // (type -> type *) or (type & -> type *).
> + if (isa<PointerType>(ToQTy)) {
> + // Only suggest taking address of L-values.
> + if (!Arg->isLValue())
> + return false;
> +
> + OpaqueValueExpr TmpExpr(Arg->getExprLoc(),
> + S.Context.getPointerType(FromQTy), VK_RValue);
> + ImplicitConversionSequence ICS =
> + TryCopyInitialization(S, &TmpExpr, ToQTy, true, true, false);
> + if (!ICS.isBad()) {
> + if (NeedParen) {
> + ConvFix.Hints.push_back(FixItHint::CreateInsertion(Begin, "&("));
> + ConvFix.Hints.push_back(FixItHint::CreateInsertion(End, ")"));
> + } else {
> + ConvFix.Hints.push_back(FixItHint::CreateInsertion(Begin, "&"));
> + }
> + ConvFix.NumConversionsFixed++;
> + if (ConvFix.NumConversionsFixed == 1)
> + ConvFix.Kind = OFIK_TakeAddress;
> + return true;
> + }
> + }
> + return false;
> +}
> +
> void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand, unsigned I) {
> const ImplicitConversionSequence &Conv = Cand->Conversions[I];
> assert(Conv.isBad());
> @@ -6903,10 +6982,21 @@
> }
>
> // TODO: specialize more based on the kind of mismatch
> - S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_conv)
> - << (unsigned) FnKind << FnDesc
> +
> + // Emit the generic diagnostic and, optionally, add the hints to it.
> + PartialDiagnostic FDiag = S.PDiag(diag::note_ovl_candidate_bad_conv);
> + FDiag << (unsigned) FnKind << FnDesc
> << (FromExpr ? FromExpr->getSourceRange() : SourceRange())
> - << FromTy << ToTy << (unsigned) isObjectArgument << I+1;
> + << FromTy << ToTy << (unsigned) isObjectArgument << I + 1
> + << (unsigned) (Cand->Fix.Kind);
> +
> + // If we can fix the conversion, suggest the FixIts.
> + for (llvm::SmallVector<FixItHint, 4>::iterator
> + HI = Cand->Fix.Hints.begin(), HE = Cand->Fix.Hints.end();
> + HI != HE; ++HI)
> + FDiag << *HI;
> + S.Diag(Fn->getLocation(), FDiag);
> +
> MaybeEmitInheritedConstructorNote(S, Fn);
> }
>
> @@ -7256,6 +7346,18 @@
> if (R->FailureKind != ovl_fail_bad_conversion)
> return true;
>
> + // The conversion that can be fixed with a smaller number of
> changes,
> + // comes first.
> + unsigned numLFixes = L->Fix.NumConversionsFixed;
> + unsigned numRFixes = R->Fix.NumConversionsFixed;
> + numLFixes = (numLFixes == 0) ? UINT_MAX : numLFixes;
> + numRFixes = (numRFixes == 0) ? UINT_MAX : numRFixes;
> + if (numLFixes != numRFixes)
> + if (numLFixes < numRFixes)
> + return true;
> + else
> + return false;
> +
> // If there's any ordering between the defined conversions...
> // FIXME: this might not be transitive.
> assert(L->Conversions.size() == R->Conversions.size());
> @@ -7300,7 +7402,7 @@
> };
>
> /// CompleteNonViableCandidate - Normally, overload resolution only
> -/// computes up to the first
> +/// computes up to the first. Produces the FixIt set if possible.
> void CompleteNonViableCandidate(Sema &S, OverloadCandidate *Cand,
> Expr **Args, unsigned NumArgs) {
> assert(!Cand->Viable);
> @@ -7308,14 +7410,21 @@
> // Don't do anything on failures other than bad conversion.
> if (Cand->FailureKind != ovl_fail_bad_conversion) return;
>
> + // We only want the FixIts if all the arguments can be corrected.
> + bool Unfixable = false;
> +
> // Skip forward to the first bad conversion.
> unsigned ConvIdx = (Cand->IgnoreObjectArgument ? 1 : 0);
> unsigned ConvCount = Cand->Conversions.size();
> while (true) {
> assert(ConvIdx != ConvCount && "no bad conversion in candidate");
> ConvIdx++;
> - if (Cand->Conversions[ConvIdx - 1].isBad())
> + if (Cand->Conversions[ConvIdx - 1].isBad()) {
> + if ((Unfixable = !TryToFixBadConversion(S, Cand->Conversions[ConvIdx
> - 1],
> + Cand->Fix)))
> + Cand->Fix.Hints.clear();
> break;
> + }
> }
>
> if (ConvIdx == ConvCount)
> @@ -7360,16 +7469,26 @@
> // Fill in the rest of the conversions.
> unsigned NumArgsInProto = Proto->getNumArgs();
> for (; ConvIdx != ConvCount; ++ConvIdx, ++ArgIdx) {
> - if (ArgIdx < NumArgsInProto)
> + if (ArgIdx < NumArgsInProto) {
> Cand->Conversions[ConvIdx]
> = TryCopyInitialization(S, Args[ArgIdx], Proto->getArgType(ArgIdx),
> SuppressUserConversions,
> /*InOverloadResolution=*/true,
> /*AllowObjCWritebackConversion=*/
> S.getLangOptions().ObjCAutoRefCount);
> + // Store the FixIt in the candidate if it exists.
> + if (!Unfixable && Cand->Conversions[ConvIdx].isBad())
> + Unfixable = !TryToFixBadConversion(S, Cand->Conversions[ConvIdx],
> + Cand->Fix);
> + }
> else
> Cand->Conversions[ConvIdx].setEllipsis();
> }
> +
> + if (Unfixable) {
> + Cand->Fix.Hints.clear();
> + Cand->Fix.NumConversionsFixed = 0;
> + }
> }
>
> } // end anonymous namespace
>
> Added: cfe/trunk/test/FixIt/fixit-function-call.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit-function-call.cpp?rev=135509&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/FixIt/fixit-function-call.cpp (added)
> +++ cfe/trunk/test/FixIt/fixit-function-call.cpp Tue Jul 19 14:49:12 2011
> @@ -0,0 +1,87 @@
> +// RUN: %clang_cc1 -fdiagnostics-parseable-fixits -x c++ %s 2> %t || true
> +// RUN: FileCheck %s < %t
> +// PR5941
> +// END.
> +
> +/* Test fixits for * and & mismatch in function arguments.
> + * Since fixits are on the notes, they cannot be applied automatically. */
> +
> +typedef int intTy;
> +typedef int intTy2;
> +
> +void f0(int *a);
> +void f1(double *a);
> +void f1(intTy &a);
> +
> +void f2(intTy2 *a) {
> +// CHECK: error: no matching function for call to 'f1
> +// CHECK: dereference the argument with *
> +// CHECK: void f1(intTy &a);
> +// CHECK: fix-it{{.*}}*(
> +// CHECK-NEXT: fix-it{{.*}})
> +// CHECK: void f1(double *a);
> + f1(a + 1);
> +
> +// This call cannot be fixed since without resulting in null pointer
> dereference.
> +// CHECK: error: no matching function for call to 'f1
> +// CHECK-NOT: take the address of the argument with &
> +// CHECK-NOT: fix-it
> + f1((int *)0);
> +}
> +
> +void f3(int &a) {
> +// CHECK: error: no matching function for call to 'f0
> +// CHECK: fix-it{{.*}}&
> + f0(a);
> +}
> +
> +
> +void m(int *a, const int *b); // match 2
> +void m(double *a, int *b); // no match
> +void m(int *a, double *b); // no match
> +void m(intTy &a, int *b); // match 1
> +
> +void mcaller(intTy2 a, int b) {
> +// CHECK: error: no matching function for call to 'm
> +// CHECK: take the address of the argument with &
> +// CHECK: fix-it{{.*}}&
> +// CHECK: take the address of the argument with &
> +// CHECK: fix-it{{.*}}&
> +// CHECK: fix-it{{.*}}&
> + m(a, b);
> +
> +// This call cannot be fixed because (a + 1) is not an l-value.
> +// CHECK: error: no matching function for call to 'm
> +// CHECK-NOT: fix-it
> + m(a + 1, b);
> +}
> +
> +// Test derived to base conversions.
> +struct A {
> + int xx;
> +};
> +
> +struct B : public A {
> + double y;
> +};
> +
> +bool br(A &a);
> +bool bp(A *a);
> +bool dv(B b);
> +
> +void dbcaller(A *ptra, B *ptrb) {
> + B b;
> +
> +// CHECK: error: no matching function for call to 'br
> +// CHECK: fix-it{{.*}}*
> + br(ptrb); // good
> +// CHECK: error: no matching function for call to 'bp
> +// CHECK: fix-it{{.*}}&
> + bp(b); // good
> +
> +// CHECK: error: no matching function for call to 'dv
> +// CHECK-NOT: fix-it
> + dv(ptra); // bad: base to derived
> +}
> +
> +// CHECK: errors generated
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110719/88a04d17/attachment.html>
More information about the cfe-commits
mailing list