On Jul 14, 2011, at 7:26 PM, Anna Zaks wrote:
> This patch is a first step towards providing FixItHints in case a function
> call is missing one/several * or & operators (See http://llvm.org/PR5941).
>
> With this fix, the error message will display the function call. It will be
> followed by the list of notes - one for each possible candidate function
> declaration. If a function call can be fixed to match the particular
> declaration, a FixItHint will be provided along with the note.
Very cool. Comments below.
> I also bumped up the number of MaxFixItHints to 6 (we need 2 FixIts per
> incorrect argument).
>
> The remaining steps are:
> 1) Use FixIts when determining the order in which the notes should be
> displayed.
> 2) Find out if the same FixIts can be used with errors other then "error: no
> matching function for call to foo".
+def note_ovl_candidate_bad_conv_fixit : Note<"candidate "
+ "%select{function|function|constructor|"
+ "function |function |constructor |"
+ "constructor (the implicit default constructor)|"
+ "constructor (the implicit copy constructor)|"
+ "constructor (the implicit move constructor)|"
+ "function (the implicit copy assignment operator)|"
+ "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; "
+ "did you miss * or & operator%s6?">;
The diagnostic should suggest either * or &, depending on what would actually
fix the code, rather than requiring the user to think about it further. This
could be done with some kind of select, e.g.,
%select{|;dereference the argument with *|;take the address of the
argument with &}7
which has the advantage of being extensible (e.g., in case we want to also
suggest things like () to turn a function expression into a function call) and
could even be tacked on to note_ovl_candidate_bad_conv rather than creating a
new diagnostic.
Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp (revision 135234)
+++ lib/Sema/SemaOverload.cpp (working copy)
@@ -6730,6 +6730,54 @@
namespace {
+/// Update the Hints with FixIts to correct the given conversion. Return true
+/// on success.
+static bool TryToFixBadConversion(const Sema &S,
+ const ImplicitConversionSequence &Conv,
+ llvm::SmallVector<FixItHint, 4> &Hints) {
+ assert(Conv.isBad());
+ 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());
+
+ // Check if the argument needs to be dereferenced
+ // (type * -> type) or (type * -> type &).
+ if (const CanQual<PointerType> FromPtrTy = FromQTy->getAs<PointerType>()) {
+ const CanQualType ToTy = ToQTy.getNonReferenceType();
+ const CanQualType FromTy = FromPtrTy->getPointeeType();
+ if (ToTy.getUnqualifiedType() == FromTy.getUnqualifiedType() &&
+ ToTy.isAtLeastAsQualifiedAs(FromTy)) {
It's probably worth loosening the type checking here slightly, to support (for
example) derived-to-base conversions and Objective-C object pointer
conversions. I suspect you'll want to factor that check out into a separate
routine, since a similar check is needed for &.
+ Hints.push_back(FixItHint::CreateInsertion(Begin, "*("));
+ Hints.push_back(FixItHint::CreateInsertion(End, ")"));
+ return true;
+ }
+ }
The parentheses are necessary only in a few cases… can we perhaps whitelist
common kinds of expressions (DeclRefExprs, ParenExprs, etc.) where the
parentheses aren't needed, and then just emit the simple "*" in those cases? We
want to try fairly hard to introduce the simplest, correct Fix-It we can.
+
+ // Check if the pointer to the argument needs to be passed
+ // (type -> type *) or (type & -> type *).
+ if (const CanQual<PointerType> ToPtrTy = ToQTy->getAs<PointerType>()) {
+ const CanQualType FromTy = FromQTy.getNonReferenceType();
+ const CanQualType ToTy = ToPtrTy->getPointeeType();
+ if (FromTy.getUnqualifiedType() == ToTy.getUnqualifiedType() &&
+ ToTy.isAtLeastAsQualifiedAs(FromTy)) {
+ Hints.push_back(FixItHint::CreateInsertion(Begin, "&("));
+ Hints.push_back(FixItHint::CreateInsertion(End, ")"));
+ return true;
+ }
+ }
+
For this case, we'll also need to check whether the expression is an lvalue
(Expr::isLValue()), so that we don't produce a bogus fix-it for something like:
void f(int*);
void g() { f(17); } // can't apply the & operator here
Your patch is looking quite good; thanks for working on this!
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
