[cfe-commits] r130878 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/Sema.cpp lib/Sema/SemaExpr.cpp test/SemaCXX/member-expr.cpp
Douglas Gregor
dgregor at apple.com
Wed May 4 16:09:49 PDT 2011
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. The idea, here, is that it would be *awesome* if it only took a few lines of code in each place we want to try this correction. Then, we could propagate this kind of recovery throughout the expression-handling code easily.
Thanks!
- Doug
More information about the cfe-commits
mailing list