[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
Anna Zaks
ganna at apple.com
Tue Jul 19 12:49:12 PDT 2011
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
More information about the cfe-commits
mailing list