From f3546eeef1eed4661b77b93b91a29be1bf5f7d0b Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Thu, 28 Jul 2011 19:46:48 +0000 Subject: [PATCH] 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 | 36 +++---- include/clang/Sema/SemaFixItUtils.h | 91 ++++++++++++++++ lib/Sema/CMakeLists.txt | 1 + lib/Sema/SemaFixItUtils.cpp | 160 ++++++++++++++++++++++++++++ lib/Sema/SemaOverload.cpp | 129 +++------------------- 5 files changed, 283 insertions(+), 134 deletions(-) create mode 100644 include/clang/Sema/SemaFixItUtils.h create mode 100644 lib/Sema/SemaFixItUtils.cpp diff --git a/include/clang/Sema/Overload.h b/include/clang/Sema/Overload.h index 4f3d3e8f6f..9d26202839 100644 --- a/include/clang/Sema/Overload.h +++ b/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 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 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++ diff --git a/include/clang/Sema/SemaFixItUtils.h b/include/clang/Sema/SemaFixItUtils.h new file mode 100644 index 0000000000..0c1bba5078 --- /dev/null +++ b/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 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 diff --git a/lib/Sema/CMakeLists.txt b/lib/Sema/CMakeLists.txt index 652198189a..3766439482 100644 --- a/lib/Sema/CMakeLists.txt +++ b/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 diff --git a/lib/Sema/SemaFixItUtils.cpp b/lib/Sema/SemaFixItUtils.cpp new file mode 100644 index 0000000000..8e8a46da73 --- /dev/null +++ b/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(From) && isa(To)) { + From = S.Context.getCanonicalType( + (cast(From))->getPointeeType()); + To = S.Context.getCanonicalType( + (cast(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(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(Expr) || + isa(FullExpr) || + isa(Expr) || + isa(Expr) || + isa(Expr)) + NeedParen = false; + + // Check if the argument needs to be dereferenced: + // (type * -> type) or (type * -> type &). + if (const PointerType *FromPtrTy = dyn_cast(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(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(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(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; +} diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index 93acf89e90..79aa305b86 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/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(Arg) || - isa(Arg) || - isa(Arg) || - isa(Arg) || - isa(Arg)) - NeedParen = false; - if (const UnaryOperator *UO = dyn_cast(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(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(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(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(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 -- 2.40.0