Addressing code review comments for commit 135509 - Add FixItHints in case a C++ function call is missing * or & operators on

llvm-svn: 135643
This commit is contained in:
Anna Zaks 2011-07-21 00:34:39 +00:00
parent 098ca0e7e6
commit 9ccf84e35d
4 changed files with 58 additions and 13 deletions

View File

@ -1594,7 +1594,9 @@ def note_ovl_candidate_bad_conv : Note<"candidate "
" not viable: no known conversion from %2 to %3 for "
"%select{%ordinal5 argument|object argument}4; "
"%select{|dereference the argument with *|"
"take the address of the argument with &}6">;
"take the address of the argument with &|"
"remove *|"
"remove &}6">;
def note_ovl_candidate_bad_arc_conv : Note<"candidate "
"%select{function|function|constructor|"
"function |function |constructor |"

View File

@ -531,7 +531,9 @@ namespace clang {
enum OverloadFixItKind {
OFIK_Undefined = 0,
OFIK_Dereference,
OFIK_TakeAddress
OFIK_TakeAddress,
OFIK_RemoveDereference,
OFIK_RemoveTakeAddress
};
/// OverloadCandidate - A single candidate in an overload set (C++ 13.3).
@ -565,7 +567,7 @@ namespace clang {
/// 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, 4> Hints;
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.

View File

@ -6752,7 +6752,14 @@ static bool TryToFixBadConversion(Sema &S,
const SourceLocation End = S.PP.getLocForEndOfToken(Arg->getSourceRange()
.getEnd());
bool NeedParen = true;
if (isa<ParenExpr>(Arg) || isa<DeclRefExpr>(Arg))
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
@ -6765,13 +6772,20 @@ static bool TryToFixBadConversion(Sema &S,
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 (NeedParen) {
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 {
@ -6779,7 +6793,7 @@ static bool TryToFixBadConversion(Sema &S,
}
ConvFix.NumConversionsFixed++;
if (ConvFix.NumConversionsFixed == 1)
ConvFix.Kind = OFIK_Dereference;
ConvFix.Kind = FixKind;
return true;
}
}
@ -6788,15 +6802,24 @@ static bool TryToFixBadConversion(Sema &S,
// (type -> type *) or (type & -> type *).
if (isa<PointerType>(ToQTy)) {
// Only suggest taking address of L-values.
if (!Arg->isLValue())
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 (NeedParen) {
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 {
@ -6804,7 +6827,7 @@ static bool TryToFixBadConversion(Sema &S,
}
ConvFix.NumConversionsFixed++;
if (ConvFix.NumConversionsFixed == 1)
ConvFix.Kind = OFIK_TakeAddress;
ConvFix.Kind = FixKind;
return true;
}
}
@ -7004,7 +7027,7 @@ void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand, unsigned I) {
<< (unsigned) (Cand->Fix.Kind);
// If we can fix the conversion, suggest the FixIts.
for (llvm::SmallVector<FixItHint, 4>::iterator
for (llvm::SmallVector<FixItHint, 1>::iterator
HI = Cand->Fix.Hints.begin(), HE = Cand->Fix.Hints.end();
HI != HE; ++HI)
FDiag << *HI;
@ -7365,11 +7388,12 @@ struct CompareOverloadCandidatesForDisplay {
unsigned numRFixes = R->Fix.NumConversionsFixed;
numLFixes = (numLFixes == 0) ? UINT_MAX : numLFixes;
numRFixes = (numRFixes == 0) ? UINT_MAX : numRFixes;
if (numLFixes != 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.

View File

@ -24,7 +24,7 @@ void f2(intTy2 *a) {
// 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: dereference the argument with *
// CHECK-NOT: fix-it
f1((int *)0);
}
@ -65,16 +65,19 @@ struct B : public A {
double y;
};
class C : A {};
bool br(A &a);
bool bp(A *a);
bool dv(B b);
void dbcaller(A *ptra, B *ptrb) {
void dbcaller(A *ptra, B *ptrb, C &c) {
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
@ -82,6 +85,20 @@ void dbcaller(A *ptra, B *ptrb) {
// CHECK: error: no matching function for call to 'dv
// CHECK-NOT: fix-it
dv(ptra); // bad: base to derived
// CHECK: error: no matching function for call to 'dv
// CHECK: remove &
dv(&b);
// CHECK: error: no matching function for call to 'bp
// CHECK: remove *
bp(*ptra);
// TODO: Test that we do not provide a fixit when inheritance is private.
// CHECK: error: no matching function for call to 'bp
// There should not be a fixit here:
// CHECK: fix-it
bp(c);
}
// CHECK: errors generated