From: Anna Zaks Date: Tue, 19 Jul 2011 19:49:12 +0000 (+0000) Subject: Add FixItHints in case a C++ function call is missing * or & operators on one/several... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b89fe6b04c5b8a2d080c1c5605b72f809fc9ee68;p=clang Add FixItHints in case a C++ function call is missing * or & operators on one/several of it's parameters (addresses http://llvm.org/PR5941). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@135509 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h index 20eebaff08..d5b53e74c7 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -615,7 +615,7 @@ private: /// 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. diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 3da830a1a0..ad022c23e6 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1592,7 +1592,9 @@ def note_ovl_candidate_bad_conv : Note<"candidate " "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 |" diff --git a/include/clang/Sema/Overload.h b/include/clang/Sema/Overload.h index 32d4cbdac1..65c7e7dd9c 100644 --- a/include/clang/Sema/Overload.h +++ b/include/clang/Sema/Overload.h @@ -528,6 +528,12 @@ namespace clang { 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 @@ namespace clang { /// function arguments to the function parameters. llvm::SmallVector 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 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; diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index 437b2b5c6d..5ed950e5c5 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -6732,6 +6732,85 @@ void ImplicitConversionSequence::DiagnoseAmbiguousConversion( 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(Arg) || isa(Arg)) + NeedParen = false; + + // Check if the argument needs to be dereferenced + // (type * -> type) or (type * -> type &). + if (const PointerType *FromPtrTy = dyn_cast(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(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 @@ void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand, unsigned I) { } // 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::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 @@ struct CompareOverloadCandidatesForDisplay { 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 @@ struct CompareOverloadCandidatesForDisplay { }; /// 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 @@ void CompleteNonViableCandidate(Sema &S, OverloadCandidate *Cand, // 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 @@ void CompleteNonViableCandidate(Sema &S, OverloadCandidate *Cand, // 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 diff --git a/test/FixIt/fixit-function-call.cpp b/test/FixIt/fixit-function-call.cpp new file mode 100644 index 0000000000..e59e21b933 --- /dev/null +++ b/test/FixIt/fixit-function-call.cpp @@ -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