Refactor the */& mismatch fixit generation out of SemaOverload and provide a simple conversion checking function.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@136376 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Sema/Overload.h

@@ -21,6 +21,7 @@
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/UnresolvedSet.h"
+#include "clang/Sema/SemaFixItUtils.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 
@@ -528,14 +529,6 @@ namespace clang {
     ovl_fail_final_conversion_not_exact
   };
 
-  enum OverloadFixItKind {
-    OFIK_Undefined = 0,
-    OFIK_Dereference,
-    OFIK_TakeAddress,
-    OFIK_RemoveDereference,
-    OFIK_RemoveTakeAddress
-  };
-
   /// OverloadCandidate - A single candidate in an overload set (C++ 13.3).
   struct OverloadCandidate {
     /// Function - The actual function that this candidate
@@ -565,19 +558,7 @@ namespace clang {
     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).
-      SmallVector<FixItHint, 1> 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;
+    ConversionFixItGenerator Fix;
 
     /// Viable - True to indicate that this overload candidate is viable.
     bool Viable;
@@ -654,6 +635,19 @@ namespace clang {
       }
       return false;
     }
+
+    bool TryToFixBadConversion(unsigned Idx, Sema &S) {
+      bool CanFix = Fix.tryToFixConversion(
+                      Conversions[Idx].Bad.FromExpr,
+                      Conversions[Idx].Bad.getFromType(),
+                      Conversions[Idx].Bad.getToType(), S);
+
+      // If at least one conversion fails, the candidate cannot be fixed.
+      if (!CanFix)
+        Fix.clear();
+
+      return CanFix;
+    }
   };
 
   /// OverloadCandidateSet - A set of overload candidates, used in C++

include/clang/Sema/SemaFixItUtils.h

@@ -0,0 +1,91 @@
+//===--- SemaFixItUtils.h - Sema FixIts -----------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines helper classes for generation of Sema FixItHints.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_SEMA_FIXITUTILS_H
+#define LLVM_CLANG_SEMA_FIXITUTILS_H
+
+#include "clang/AST/Expr.h"
+
+namespace clang {
+
+enum OverloadFixItKind {
+  OFIK_Undefined = 0,
+  OFIK_Dereference,
+  OFIK_TakeAddress,
+  OFIK_RemoveDereference,
+  OFIK_RemoveTakeAddress
+};
+
+class Sema;
+
+/// The class facilities generation and storage of conversion FixIts. Hints for
+/// new conversions are added using TryToFixConversion method. The default type
+/// conversion checker can be reset.
+struct ConversionFixItGenerator {
+  /// Performs a simple check to see if From type can be converted to To type.
+  static bool compareTypesSimple(CanQualType From,
+                                 CanQualType To,
+                                 Sema &S,
+                                 SourceLocation Loc,
+                                 ExprValueKind FromVK);
+
+  /// The list of Hints generated so far.
+  SmallVector<FixItHint, 1> 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. If multiple conversions are fixed, corresponds
+  /// to the kid of the very first conversion.
+  OverloadFixItKind Kind;
+
+  typedef bool (*TypeComparisonFuncTy) (const CanQualType FromTy,
+                                        const CanQualType ToTy,
+                                        Sema &S,
+                                        SourceLocation Loc,
+                                        ExprValueKind FromVK);
+  /// The type comparison function used to decide if expression FromExpr of
+  /// type FromTy can be converted to ToTy. For example, one could check if
+  /// an implicit conversion exists. Returns true if comparison exists.
+  TypeComparisonFuncTy CompareTypes;
+
+  ConversionFixItGenerator(TypeComparisonFuncTy Foo): NumConversionsFixed(0),
+                                                      Kind(OFIK_Undefined),
+                                                      CompareTypes(Foo) {}
+
+  ConversionFixItGenerator(): NumConversionsFixed(0),
+                              Kind(OFIK_Undefined),
+                              CompareTypes(compareTypesSimple) {}
+
+  /// Resets the default conversion checker method.
+  void setConversionChecker(TypeComparisonFuncTy Foo) {
+    CompareTypes = Foo;
+  }
+
+  /// If possible, generates and stores a fix for the given conversion.
+  bool tryToFixConversion(const Expr *FromExpr,
+                          const QualType FromQTy, const QualType ToQTy,
+                          Sema &S);
+
+  void clear() {
+    Hints.clear();
+    NumConversionsFixed = 0;
+  }
+
+  bool isNull() {
+    return (NumConversionsFixed == 0);
+  }
+};
+
+} // endof namespace clang
+#endif // LLVM_CLANG_SEMA_FIXITUTILS_H

lib/Sema/CMakeLists.txt

@@ -25,6 +25,7 @@ add_clang_library(clangSema
   SemaExprCXX.cpp
   SemaExprMember.cpp
   SemaExprObjC.cpp
+  SemaFixItUtils.cpp
   SemaInit.cpp
   SemaLookup.cpp
   SemaObjCProperty.cpp

lib/Sema/SemaFixItUtils.cpp

@@ -0,0 +1,160 @@
+//===--- SemaFixItUtils.cpp - Sema FixIts ---------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines helper classes for generation of Sema FixItHints.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprObjC.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaFixItUtils.h"
+
+using namespace clang;
+
+bool ConversionFixItGenerator::compareTypesSimple(CanQualType From,
+                                                  CanQualType To,
+                                                  Sema &S,
+                                                  SourceLocation Loc,
+                                                  ExprValueKind FromVK) {
+  if (!To.isAtLeastAsQualifiedAs(From))
+    return false;
+
+  From = From.getNonReferenceType();
+  To = To.getNonReferenceType();
+
+  // If both are pointer types, work with the pointee types.
+  if (isa<PointerType>(From) && isa<PointerType>(To)) {
+    From = S.Context.getCanonicalType(
+        (cast<PointerType>(From))->getPointeeType());
+    To = S.Context.getCanonicalType(
+        (cast<PointerType>(To))->getPointeeType());
+  }
+
+  const CanQualType FromUnq = From.getUnqualifiedType();
+  const CanQualType ToUnq = To.getUnqualifiedType();
+
+  if ((FromUnq == ToUnq || (S.IsDerivedFrom(FromUnq, ToUnq)) ) &&
+      To.isAtLeastAsQualifiedAs(From))
+    return true;
+  return false;
+}
+
+bool ConversionFixItGenerator::tryToFixConversion(const Expr *FullExpr,
+                                                  const QualType FromTy,
+                                                  const QualType ToTy,
+                                                  Sema &S) {
+  if (!FullExpr)
+    return false;
+
+  const CanQualType FromQTy = S.Context.getCanonicalType(FromTy);
+  const CanQualType ToQTy = S.Context.getCanonicalType(ToTy);
+  const SourceLocation Begin = FullExpr->getSourceRange().getBegin();
+  const SourceLocation End = S.PP.getLocForEndOfToken(FullExpr->getSourceRange()
+                                                      .getEnd());
+
+  // Strip the implicit casts - those are implied by the compiler, not the
+  // original source code.
+  const Expr* Expr = FullExpr->IgnoreImpCasts();
+
+  bool NeedParen = true;
+  if (isa<ArraySubscriptExpr>(Expr) ||
+      isa<CallExpr>(Expr) ||
+      isa<DeclRefExpr>(Expr) ||
+      isa<CastExpr>(Expr) ||
+      isa<CXXNewExpr>(Expr) ||
+      isa<CXXConstructExpr>(Expr) ||
+      isa<CXXDeleteExpr>(Expr) ||
+      isa<CXXNoexceptExpr>(Expr) ||
+      isa<CXXPseudoDestructorExpr>(Expr) ||
+      isa<CXXScalarValueInitExpr>(Expr) ||
+      isa<CXXThisExpr>(Expr) ||
+      isa<CXXTypeidExpr>(Expr) ||
+      isa<CXXUnresolvedConstructExpr>(Expr) ||
+      isa<ObjCMessageExpr>(Expr) ||
+      isa<ObjCPropertyRefExpr>(Expr) ||
+      isa<ObjCProtocolExpr>(Expr) ||
+      isa<MemberExpr>(Expr) ||
+      isa<ParenExpr>(FullExpr) ||
+      isa<ParenListExpr>(Expr) ||
+      isa<SizeOfPackExpr>(Expr) ||
+      isa<UnaryOperator>(Expr))
+    NeedParen = false;
+
+  // Check if the argument needs to be dereferenced:
+  //   (type * -> type) or (type * -> type &).
+  if (const PointerType *FromPtrTy = dyn_cast<PointerType>(FromQTy)) {
+    OverloadFixItKind FixKind = OFIK_Dereference;
+
+    bool CanConvert = CompareTypes(
+      S.Context.getCanonicalType(FromPtrTy->getPointeeType()), ToQTy,
+                                 S, Begin, VK_LValue);
+    if (CanConvert) {
+      // Do not suggest dereferencing a Null pointer.
+      if (Expr->IgnoreParenCasts()->
+          isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull))
+        return false;
+
+      if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Expr)) {
+        if (UO->getOpcode() == UO_AddrOf) {
+          FixKind = OFIK_RemoveTakeAddress;
+          Hints.push_back(FixItHint::CreateRemoval(
+                            CharSourceRange::getTokenRange(Begin, Begin)));
+        }
+      } else if (NeedParen) {
+        Hints.push_back(FixItHint::CreateInsertion(Begin, "*("));
+        Hints.push_back(FixItHint::CreateInsertion(End, ")"));
+      } else {
+        Hints.push_back(FixItHint::CreateInsertion(Begin, "*"));
+      }
+
+      NumConversionsFixed++;
+      if (NumConversionsFixed == 1)
+        Kind = FixKind;
+      return true;
+    }
+  }
+
+  // Check if the pointer to the argument needs to be passed:
+  //   (type -> type *) or (type & -> type *).
+  if (isa<PointerType>(ToQTy)) {
+    bool CanConvert = false;
+    OverloadFixItKind FixKind = OFIK_TakeAddress;
+
+    // Only suggest taking address of L-values.
+    if (!Expr->isLValue() || Expr->getObjectKind() != OK_Ordinary)
+      return false;
+
+    CanConvert = CompareTypes(S.Context.getPointerType(FromQTy), ToQTy,
+                              S, Begin, VK_RValue);
+    if (CanConvert) {
+
+      if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Expr)) {
+        if (UO->getOpcode() == UO_Deref) {
+          FixKind = OFIK_RemoveDereference;
+          Hints.push_back(FixItHint::CreateRemoval(
+                            CharSourceRange::getTokenRange(Begin, Begin)));
+        }
+      } else if (NeedParen) {
+        Hints.push_back(FixItHint::CreateInsertion(Begin, "&("));
+        Hints.push_back(FixItHint::CreateInsertion(End, ")"));
+      } else {
+        Hints.push_back(FixItHint::CreateInsertion(Begin, "&"));
+      }
+
+      NumConversionsFixed++;
+      if (NumConversionsFixed == 1)
+        Kind = FixKind;
+      return true;
+    }
+  }
+
+  return false;
+}

lib/Sema/SemaOverload.cpp

@@ -3644,6 +3644,18 @@ TryCopyInitialization(Sema &S, Expr *From, QualType ToType,
                                AllowObjCWritebackConversion);
 }
 
+static bool TryCopyInitialization(const CanQualType FromQTy,
+                                  const CanQualType ToQTy,
+                                  Sema &S,
+                                  SourceLocation Loc,
+                                  ExprValueKind FromVK) {
+  OpaqueValueExpr TmpExpr(Loc, FromQTy, FromVK);
+  ImplicitConversionSequence ICS =
+    TryCopyInitialization(S, &TmpExpr, ToQTy, true, true, false);
+
+  return !ICS.isBad();
+}
+
 /// TryObjectArgumentInitialization - Try to initialize the object
 /// parameter of the given member function (@c Method) from the
 /// expression @p From.
@@ -6729,108 +6741,6 @@ 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) ||
-      isa<ArraySubscriptExpr>(Arg) ||
-      isa<CallExpr>(Arg) ||
-      isa<MemberExpr>(Arg))
-    NeedParen = false;
-  if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Arg))
-    if (UO->isPostfix())
-      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);
-
-    OverloadFixItKind FixKind = OFIK_Dereference;
-    if (!ICS.isBad()) {
-      // Do not suggest dereferencing a Null pointer.
-      if (Arg->IgnoreParenCasts()->
-          isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull))
-        return false;
-
-      if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Arg)) {
-        if (UO->getOpcode() == UO_AddrOf) {
-          ConvFix.Hints.push_back(
-                               FixItHint::CreateRemoval(Arg->getSourceRange()));
-          FixKind = OFIK_RemoveTakeAddress;
-        }
-      } else 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 = FixKind;
-      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() || Arg->getObjectKind() != OK_Ordinary)
-      return false;
-
-    OpaqueValueExpr TmpExpr(Arg->getExprLoc(),
-                            S.Context.getPointerType(FromQTy), VK_RValue);
-    ImplicitConversionSequence ICS =
-      TryCopyInitialization(S, &TmpExpr, ToQTy, true, true, false);
-
-    OverloadFixItKind FixKind = OFIK_TakeAddress;
-    if (!ICS.isBad()) {
-
-      if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Arg)) {
-        if (UO->getOpcode() == UO_Deref) {
-          ConvFix.Hints.push_back(
-                               FixItHint::CreateRemoval(Arg->getSourceRange()));
-          FixKind = OFIK_RemoveDereference;
-        }
-      } else 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 = FixKind;
-      return true;
-    }
-  }
-  return false;
-}
-
 void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand, unsigned I) {
   const ImplicitConversionSequence &Conv = Cand->Conversions[I];
   assert(Conv.isBad());
@@ -7015,7 +6925,6 @@ void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand, unsigned I) {
       }
   }
   
-  // TODO: specialize more based on the kind of mismatch
   // 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
@@ -7446,6 +7355,8 @@ void CompleteNonViableCandidate(Sema &S, OverloadCandidate *Cand,
 
   // We only want the FixIts if all the arguments can be corrected.
   bool Unfixable = false;
+  // Use a implicit copy initialization to check conversion fixes.
+  Cand->Fix.setConversionChecker(TryCopyInitialization);
 
   // Skip forward to the first bad conversion.
   unsigned ConvIdx = (Cand->IgnoreObjectArgument ? 1 : 0);
@@ -7454,9 +7365,7 @@ void CompleteNonViableCandidate(Sema &S, OverloadCandidate *Cand,
     assert(ConvIdx != ConvCount && "no bad conversion in candidate");
     ConvIdx++;
     if (Cand->Conversions[ConvIdx - 1].isBad()) {
-      if ((Unfixable = !TryToFixBadConversion(S, Cand->Conversions[ConvIdx - 1],
-                                                 Cand->Fix)))
-        Cand->Fix.Hints.clear();
+      Unfixable = !Cand->TryToFixBadConversion(ConvIdx - 1, S);
       break;
     }
   }
@@ -7512,17 +7421,11 @@ void CompleteNonViableCandidate(Sema &S, OverloadCandidate *Cand,
                                   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);
+        Unfixable = !Cand->TryToFixBadConversion(ConvIdx, S);
     }
     else
       Cand->Conversions[ConvIdx].setEllipsis();
   }
-
-  if (Unfixable) {
-    Cand->Fix.Hints.clear();
-    Cand->Fix.NumConversionsFixed = 0;
-  }
 }
 
 } // end anonymous namespace