On Wed, May 4, 2011 at 16:09, Douglas Gregor <[email protected]> wrote: > > On May 4, 2011, at 3:10 PM, Matt Beaumont-Gay wrote: > >> Author: matthewbg >> Date: Wed May 4 17:10:40 2011 >> New Revision: 130878 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=130878&view=rev >> Log: >> Implement Sema::isExprCallable. >> >> We can use this to produce nice diagnostics (and try to fixit-and-recover) in >> various cases where we might see "MyFunction" instead of "MyFunction()". The >> changes in SemaExpr are an example of how to use isExprCallable. >> >> Modified: >> cfe/trunk/include/clang/Sema/Sema.h >> cfe/trunk/lib/Sema/Sema.cpp >> cfe/trunk/lib/Sema/SemaExpr.cpp >> cfe/trunk/test/SemaCXX/member-expr.cpp >> >> Modified: cfe/trunk/include/clang/Sema/Sema.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=130878&r1=130877&r2=130878&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Sema/Sema.h (original) >> +++ cfe/trunk/include/clang/Sema/Sema.h Wed May 4 17:10:40 2011 >> @@ -2074,7 +2074,14 @@ >> void MarkDeclarationReferenced(SourceLocation Loc, Decl *D); >> void MarkDeclarationsReferencedInType(SourceLocation Loc, QualType T); >> void MarkDeclarationsReferencedInExpr(Expr *E); >> - >> + >> + /// \brief Figure out if an expression could be turned into a call. >> + bool isExprCallable(const Expr &E, QualType &ZeroArgCallReturnTy, >> + UnresolvedSetImpl &NonTemplateOverloads); >> + /// \brief Give notes for a set of overloads. >> + void NoteOverloads(const UnresolvedSetImpl &Overloads, >> + const SourceLocation FinalNoteLoc); >> + >> /// \brief Conditionally issue a diagnostic based on the current >> /// evaluation context. >> /// >> >> Modified: cfe/trunk/lib/Sema/Sema.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=130878&r1=130877&r2=130878&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/Sema.cpp (original) >> +++ cfe/trunk/lib/Sema/Sema.cpp Wed May 4 17:10:40 2011 >> @@ -31,6 +31,7 @@ >> #include "clang/AST/DeclCXX.h" >> #include "clang/AST/DeclObjC.h" >> #include "clang/AST/Expr.h" >> +#include "clang/AST/ExprCXX.h" >> #include "clang/AST/StmtCXX.h" >> #include "clang/Lex/Preprocessor.h" >> #include "clang/Basic/PartialDiagnostic.h" >> @@ -766,3 +767,102 @@ >> >> OS << '\n'; >> } >> + >> +/// \brief Figure out if an expression could be turned into a call. >> +/// >> +/// Use this when trying to recover from an error where the programmer may >> have >> +/// written just the name of a function instead of actually calling it. >> +/// >> +/// \param E - The expression to examine. >> +/// \param ZeroArgCallReturnTy - If the expression can be turned into a call >> +/// with no arguments, this parameter is set to the type returned by such a >> +/// call; otherwise, it is set to an empty QualType. >> +/// \param NonTemplateOverloads - If the expression is an overloaded >> function >> +/// name, this parameter is populated with the decls of the various >> overloads. >> +bool Sema::isExprCallable(const Expr &E, QualType &ZeroArgCallReturnTy, >> + UnresolvedSetImpl &NonTemplateOverloads) { >> + ZeroArgCallReturnTy = QualType(); >> + NonTemplateOverloads.clear(); >> + if (const OverloadExpr *Overloads = dyn_cast<OverloadExpr>(&E)) { >> + for (OverloadExpr::decls_iterator it = Overloads->decls_begin(), >> + DeclsEnd = Overloads->decls_end(); it != DeclsEnd; ++it) { >> + // Our overload set may include TemplateDecls, which we'll ignore for >> our >> + // present purpose. >> + if (const FunctionDecl *OverloadDecl = dyn_cast<FunctionDecl>(*it)) { >> + NonTemplateOverloads.addDecl(*it); >> + if (OverloadDecl->getMinRequiredArguments() == 0) >> + ZeroArgCallReturnTy = OverloadDecl->getResultType(); >> + } >> + } >> + return true; >> + } >> + >> + if (const DeclRefExpr *DeclRef = dyn_cast<DeclRefExpr>(&E)) { >> + if (const FunctionDecl *Fun = >> dyn_cast<FunctionDecl>(DeclRef->getDecl())) { >> + if (Fun->getMinRequiredArguments() == 0) >> + ZeroArgCallReturnTy = Fun->getResultType(); >> + return true; >> + } >> + } >> + >> + // We don't have an expression that's convenient to get a FunctionDecl >> from, >> + // but we can at least check if the type is "function of 0 arguments". >> + QualType ExprTy = E.getType(); >> + const FunctionType *FunTy = NULL; >> + if (const PointerType *Ptr = ExprTy->getAs<PointerType>()) >> + FunTy = Ptr->getPointeeType()->getAs<FunctionType>(); >> + else if (const ReferenceType *Ref = ExprTy->getAs<ReferenceType>()) >> + FunTy = Ref->getPointeeType()->getAs<FunctionType>(); > > It would be nice to handle BlockPointerTypes here, too. But if you don't want > to do that, it's okay. > >> + if (!FunTy) >> + FunTy = ExprTy->getAs<FunctionType>(); >> + if (!FunTy && ExprTy == Context.BoundMemberTy) { >> + // Look for the bound-member type. If it's still overloaded, give up, >> + // although we probably should have fallen into the OverloadExpr case >> above >> + // if we actually have an overloaded bound member. >> + QualType BoundMemberTy = Expr::findBoundMemberType(&E); >> + if (!BoundMemberTy.isNull()) >> + FunTy = BoundMemberTy->castAs<FunctionType>(); >> + } >> + >> + if (const FunctionProtoType *FPT = >> + dyn_cast_or_null<FunctionProtoType>(FunTy)) { >> + if (FPT->getNumArgs() == 0) >> + ZeroArgCallReturnTy = FunTy->getResultType(); >> + return true; >> + } >> + return false; >> +} >> + >> +/// \brief Give notes for a set of overloads. >> +/// >> +/// A companion to isExprCallable. In cases when the name that the >> programmer >> +/// wrote was an overloaded function, we may be able to make some guesses >> about >> +/// plausible overloads based on their return types; such guesses can be >> handed >> +/// off to this method to be emitted as notes. >> +/// >> +/// \param Overloads - The overloads to note. >> +/// \param FinalNoteLoc - If we've suppressed printing some overloads due to >> +/// -fshow-overloads=best, this is the location to attach to the note >> about too >> +/// many candidates. Typically this will be the location of the original >> +/// ill-formed expression. >> +void Sema::NoteOverloads(const UnresolvedSetImpl &Overloads, >> + const SourceLocation FinalNoteLoc) { >> + int ShownOverloads = 0; >> + int SuppressedOverloads = 0; >> + for (UnresolvedSetImpl::iterator It = Overloads.begin(), >> + DeclsEnd = Overloads.end(); It != DeclsEnd; ++It) { >> + // FIXME: Magic number for max shown overloads stolen from >> + // OverloadCandidateSet::NoteCandidates. >> + if (ShownOverloads >= 4 && >> + Diags.getShowOverloads() == Diagnostic::Ovl_Best) { >> + ++SuppressedOverloads; >> + continue; >> + } >> + Diag(cast<FunctionDecl>(*It)->getSourceRange().getBegin(), >> + diag::note_member_ref_possible_intended_overload); >> + ++ShownOverloads; >> + } >> + if (SuppressedOverloads) >> + Diag(FinalNoteLoc, diag::note_ovl_too_many_candidates) >> + << SuppressedOverloads; >> +} >> >> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=130878&r1=130877&r2=130878&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed May 4 17:10:40 2011 >> @@ -4377,120 +4377,52 @@ >> >> // If the user is trying to apply -> or . to a function name, it's probably >> // because they forgot parentheses to call that function. >> - bool TryCall = false; >> - bool Overloaded = false; >> - UnresolvedSet<8> AllOverloads; >> - if (const OverloadExpr *Overloads = >> dyn_cast<OverloadExpr>(BaseExpr.get())) { >> - AllOverloads.append(Overloads->decls_begin(), Overloads->decls_end()); >> - TryCall = true; >> - Overloaded = true; >> - } else if (DeclRefExpr *DeclRef = dyn_cast<DeclRefExpr>(BaseExpr.get())) { >> - if (FunctionDecl* Fun = dyn_cast<FunctionDecl>(DeclRef->getDecl())) { >> - AllOverloads.addDecl(Fun); >> - TryCall = true; >> - } >> - } >> - >> - if (TryCall) { >> - // Plunder the overload set for something that would make the member >> - // expression valid. >> - UnresolvedSet<4> ViableOverloads; >> - bool HasViableZeroArgOverload = false; >> - for (OverloadExpr::decls_iterator it = AllOverloads.begin(), >> - DeclsEnd = AllOverloads.end(); it != DeclsEnd; ++it) { >> - // Our overload set may include TemplateDecls, which we'll ignore for >> the >> - // purposes of determining whether we can issue a '()' fixit. >> - if (const FunctionDecl *OverloadDecl = dyn_cast<FunctionDecl>(*it)) { >> - QualType ResultTy = OverloadDecl->getResultType(); >> - if ((!IsArrow && ResultTy->isRecordType()) || >> - (IsArrow && ResultTy->isPointerType() && >> - ResultTy->getPointeeType()->isRecordType())) { >> - ViableOverloads.addDecl(*it); >> - if (OverloadDecl->getMinRequiredArguments() == 0) { >> - HasViableZeroArgOverload = true; >> - } >> - } >> - } >> - } >> - >> - if (!HasViableZeroArgOverload || ViableOverloads.size() != 1) { >> + QualType ZeroArgCallTy; >> + UnresolvedSet<4> Overloads; >> + if (isExprCallable(*BaseExpr.get(), ZeroArgCallTy, Overloads)) { >> + if (ZeroArgCallTy.isNull()) { >> Diag(BaseExpr.get()->getExprLoc(), >> diag::err_member_reference_needs_call) >> - << (AllOverloads.size() > 1) << 0 >> - << BaseExpr.get()->getSourceRange(); >> - int ViableOverloadCount = ViableOverloads.size(); >> - int I; >> - for (I = 0; I < ViableOverloadCount; ++I) { >> - // FIXME: Magic number for max shown overloads stolen from >> - // OverloadCandidateSet::NoteCandidates. >> - if (I >= 4 && Diags.getShowOverloads() == Diagnostic::Ovl_Best) { >> - break; >> - } >> - Diag(ViableOverloads[I].getDecl()->getSourceRange().getBegin(), >> - diag::note_member_ref_possible_intended_overload); >> - } >> - if (I != ViableOverloadCount) { >> - Diag(BaseExpr.get()->getExprLoc(), >> diag::note_ovl_too_many_candidates) >> - << int(ViableOverloadCount - I); >> + << (Overloads.size() > 1) << 0 << >> BaseExpr.get()->getSourceRange(); >> + UnresolvedSet<2> PlausibleOverloads; >> + for (OverloadExpr::decls_iterator It = Overloads.begin(), >> + DeclsEnd = Overloads.end(); It != DeclsEnd; ++It) { >> + const FunctionDecl *OverloadDecl = cast<FunctionDecl>(*It); >> + QualType OverloadResultTy = OverloadDecl->getResultType(); >> + if ((!IsArrow && OverloadResultTy->isRecordType()) || >> + (IsArrow && OverloadResultTy->isPointerType() && >> + OverloadResultTy->getPointeeType()->isRecordType())) >> + PlausibleOverloads.addDecl(It.getDecl()); >> } >> + NoteOverloads(PlausibleOverloads, BaseExpr.get()->getExprLoc()); >> return ExprError(); >> } >> - } else { >> - // We don't have an expression that's convenient to get a Decl from, >> but we >> - // can at least check if the type is "function of 0 arguments which >> returns >> - // an acceptable type". >> - const FunctionType *Fun = NULL; >> - if (const PointerType *Ptr = BaseType->getAs<PointerType>()) { >> - if ((Fun = Ptr->getPointeeType()->getAs<FunctionType>())) { >> - TryCall = true; >> - } >> - } else if ((Fun = BaseType->getAs<FunctionType>())) { >> - TryCall = true; >> - } else if (BaseType == Context.BoundMemberTy) { >> - // Look for the bound-member type. If it's still overloaded, >> - // give up, although we probably should have fallen into the >> - // OverloadExpr case above if we actually have an overloaded >> - // bound member. >> - QualType fnType = Expr::findBoundMemberType(BaseExpr.get()); >> - if (!fnType.isNull()) { >> - TryCall = true; >> - Fun = fnType->castAs<FunctionType>(); >> - } >> - } >> - >> - if (TryCall) { >> - if (const FunctionProtoType *FPT = dyn_cast<FunctionProtoType>(Fun)) { >> - if (FPT->getNumArgs() == 0) { >> - QualType ResultTy = Fun->getResultType(); >> - TryCall = (!IsArrow && ResultTy->isRecordType()) || >> - (IsArrow && ResultTy->isPointerType() && >> - ResultTy->getPointeeType()->isRecordType()); >> - } >> - } >> + if ((!IsArrow && ZeroArgCallTy->isRecordType()) || >> + (IsArrow && ZeroArgCallTy->isPointerType() && >> + ZeroArgCallTy->getPointeeType()->isRecordType())) { >> + // At this point, we know BaseExpr looks like it's potentially >> callable >> + // with 0 arguments, and that it returns something of a reasonable >> type, >> + // so we can emit a fixit and carry on pretending that BaseExpr was >> + // actually a CallExpr. >> + SourceLocation ParenInsertionLoc = >> + PP.getLocForEndOfToken(BaseExpr.get()->getLocEnd()); >> + Diag(BaseExpr.get()->getExprLoc(), >> diag::err_member_reference_needs_call) >> + << (Overloads.size() > 1) << 1 << BaseExpr.get()->getSourceRange() >> + << FixItHint::CreateInsertion(ParenInsertionLoc, "()"); >> + // FIXME: Try this before emitting the fixit, and suppress diagnostics >> + // while doing so. >> + ExprResult NewBase = >> + ActOnCallExpr(0, BaseExpr.take(), ParenInsertionLoc, >> + MultiExprArg(*this, 0, 0), >> + ParenInsertionLoc.getFileLocWithOffset(1)); >> + if (NewBase.isInvalid()) >> + return ExprError(); >> + BaseExpr = NewBase; >> + BaseExpr = DefaultFunctionArrayConversion(BaseExpr.take()); >> + return LookupMemberExpr(R, BaseExpr, IsArrow, OpLoc, SS, >> + ObjCImpDecl, HasTemplateArgs); >> } >> } > > This is definitely a big improvement! I think we should push toward moving > yet more logic into isExprCallable(), so that one routine does all of the > magic to determine if there's a good candidate function to call and then > actually form the call, returning the resulting expression. It would need to > take some kind of function object that takes in the ZeroArgCallTy type and > determines whether they type is interesting.
Yes, that's exactly what I was planning when I originally started thinking about this. If only we had lambdas in this language... ;) -Matt _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
