Add FixItHints in case a C++ function call is missing * or & operators on one/several of it's parameters (addresses http://llvm.org/PR5941).

llvm-svn: 135509

clang/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.

clang/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 |"

clang/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<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;
 

clang/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<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 @@ 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<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 @@ 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

clang/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