r205653 - Try harder about not suggesting methods as corrections when they

Richard Smith richard at metafoo.co.uk
Sun Apr 13 15:57:44 PDT 2014


On Fri, Apr 4, 2014 at 3:16 PM, Kaelyn Takata <rikka at google.com> wrote:

> Author: rikka
> Date: Fri Apr  4 17:16:30 2014
> New Revision: 205653
>
> URL: http://llvm.org/viewvc/llvm-project?rev=205653&view=rev
> Log:
> Try harder about not suggesting methods as corrections when they
> obviously won't work. Specifically, don't suggest methods (static or
> not) from unrelated classes when the expression is a method call
> through a specific object.
>

Why only member functions? This seems like the right logic for data members
too.


> Modified:
>     cfe/trunk/include/clang/Sema/TypoCorrection.h
>     cfe/trunk/lib/Sema/SemaExpr.cpp
>     cfe/trunk/lib/Sema/SemaLookup.cpp
>     cfe/trunk/lib/Sema/SemaOverload.cpp
>     cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp
>
> Modified: cfe/trunk/include/clang/Sema/TypoCorrection.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/TypoCorrection.h?rev=205653&r1=205652&r2=205653&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/TypoCorrection.h (original)
> +++ cfe/trunk/include/clang/Sema/TypoCorrection.h Fri Apr  4 17:16:30 2014
> @@ -306,15 +306,15 @@ class FunctionCallFilterCCC : public Cor
>  public:
>    FunctionCallFilterCCC(Sema &SemaRef, unsigned NumArgs,
>                          bool HasExplicitTemplateArgs,
> -                        bool AllowNonStaticMethods = true);
> +                        MemberExpr *ME = 0);
>
>    bool ValidateCandidate(const TypoCorrection &candidate) override;
>
>   private:
>    unsigned NumArgs;
>    bool HasExplicitTemplateArgs;
> -  bool AllowNonStaticMethods;
>    DeclContext *CurContext;
> +  MemberExpr *MemberFn;
>  };
>
>  // @brief Callback class that effectively disabled typo correction
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=205653&r1=205652&r2=205653&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Apr  4 17:16:30 2014
> @@ -3974,8 +3974,8 @@ namespace {
>  class FunctionCallCCC : public FunctionCallFilterCCC {
>  public:
>    FunctionCallCCC(Sema &SemaRef, const IdentifierInfo *FuncName,
> -                  unsigned NumArgs, bool HasExplicitTemplateArgs)
> -      : FunctionCallFilterCCC(SemaRef, NumArgs, HasExplicitTemplateArgs),
> +                  unsigned NumArgs, MemberExpr *ME)
> +      : FunctionCallFilterCCC(SemaRef, NumArgs, false, ME),
>          FunctionName(FuncName) {}
>
>    bool ValidateCandidate(const TypoCorrection &candidate) override {
> @@ -3992,17 +3992,20 @@ private:
>  };
>  }
>
> -static TypoCorrection TryTypoCorrectionForCall(Sema &S,
> -                                               DeclarationNameInfo
> FuncName,
> +static TypoCorrection TryTypoCorrectionForCall(Sema &S, Expr *Fn,
> +                                               FunctionDecl *FDecl,
>                                                 ArrayRef<Expr *> Args) {
> -  FunctionCallCCC CCC(S, FuncName.getName().getAsIdentifierInfo(),
> -                      Args.size(), false);
> -  if (TypoCorrection Corrected =
> -          S.CorrectTypo(FuncName, Sema::LookupOrdinaryName,
> -                        S.getScopeForContext(S.CurContext), NULL, CCC)) {
> +  MemberExpr *ME = dyn_cast<MemberExpr>(Fn);
> +  DeclarationName FuncName = FDecl->getDeclName();
> +  SourceLocation NameLoc = ME ? ME->getMemberLoc() : Fn->getLocStart();
> +  FunctionCallCCC CCC(S, FuncName.getAsIdentifierInfo(), Args.size(), ME);
> +
> +  if (TypoCorrection Corrected = S.CorrectTypo(
> +          DeclarationNameInfo(FuncName, NameLoc),
> Sema::LookupOrdinaryName,
> +          S.getScopeForContext(S.CurContext), NULL, CCC)) {
>      if (NamedDecl *ND = Corrected.getCorrectionDecl()) {
>        if (Corrected.isOverloaded()) {
> -        OverloadCandidateSet OCS(FuncName.getLoc());
> +        OverloadCandidateSet OCS(NameLoc);
>          OverloadCandidateSet::iterator Best;
>          for (TypoCorrection::decl_iterator CD = Corrected.begin(),
>                                             CDEnd = Corrected.end();
> @@ -4011,7 +4014,7 @@ static TypoCorrection TryTypoCorrectionF
>              S.AddOverloadCandidate(FD, DeclAccessPair::make(FD, AS_none),
> Args,
>                                     OCS);
>          }
> -        switch (OCS.BestViableFunction(S, FuncName.getLoc(), Best)) {
> +        switch (OCS.BestViableFunction(S, NameLoc, Best)) {
>          case OR_Success:
>            ND = Best->Function;
>            Corrected.setCorrectionDecl(ND);
> @@ -4062,13 +4065,8 @@ Sema::ConvertArgumentsForCall(CallExpr *
>    // arguments for the remaining parameters), don't make the call.
>    if (Args.size() < NumParams) {
>      if (Args.size() < MinArgs) {
> -      MemberExpr *ME = dyn_cast<MemberExpr>(Fn);
>        TypoCorrection TC;
> -      if (FDecl && (TC = TryTypoCorrectionForCall(
> -                        *this, DeclarationNameInfo(FDecl->getDeclName(),
> -                                                   (ME ?
> ME->getMemberLoc()
> -                                                       :
> Fn->getLocStart())),
> -                        Args))) {
> +      if (FDecl && (TC = TryTypoCorrectionForCall(*this, Fn, FDecl,
> Args))) {
>          unsigned diag_id =
>              MinArgs == NumParams && !Proto->isVariadic()
>                  ? diag::err_typecheck_call_too_few_args_suggest
> @@ -4103,13 +4101,8 @@ Sema::ConvertArgumentsForCall(CallExpr *
>    // them.
>    if (Args.size() > NumParams) {
>      if (!Proto->isVariadic()) {
> -      MemberExpr *ME = dyn_cast<MemberExpr>(Fn);
>        TypoCorrection TC;
> -      if (FDecl && (TC = TryTypoCorrectionForCall(
> -                        *this, DeclarationNameInfo(FDecl->getDeclName(),
> -                                                   (ME ?
> ME->getMemberLoc()
> -                                                       :
> Fn->getLocStart())),
> -                        Args))) {
> +      if (FDecl && (TC = TryTypoCorrectionForCall(*this, Fn, FDecl,
> Args))) {
>          unsigned diag_id =
>              MinArgs == NumParams && !Proto->isVariadic()
>                  ? diag::err_typecheck_call_too_many_args_suggest
>
> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=205653&r1=205652&r2=205653&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Apr  4 17:16:30 2014
> @@ -4511,10 +4511,9 @@ bool CorrectionCandidateCallback::Valida
>
>  FunctionCallFilterCCC::FunctionCallFilterCCC(Sema &SemaRef, unsigned
> NumArgs,
>                                               bool HasExplicitTemplateArgs,
> -                                             bool AllowNonStaticMethods)
> +                                             MemberExpr *ME)
>      : NumArgs(NumArgs), HasExplicitTemplateArgs(HasExplicitTemplateArgs),
> -      AllowNonStaticMethods(AllowNonStaticMethods),
> -      CurContext(SemaRef.CurContext) {
> +      CurContext(SemaRef.CurContext), MemberFn(ME) {
>    WantTypeSpecifiers = SemaRef.getLangOpts().CPlusPlus;
>    WantRemainingKeywords = false;
>  }
> @@ -4550,13 +4549,16 @@ bool FunctionCallFilterCCC::ValidateCand
>                   FD->getMinRequiredArguments() <= NumArgs))
>        continue;
>
> -    // If the current candidate is a non-static C++ method and non-static
> -    // methods are being excluded, then skip the candidate unless the
> current
> -    // DeclContext is a method in the same class or a descendent class of
> the
> -    // candidate's parent class.
> +    // If the current candidate is a non-static C++ method, skip the
> candidate
> +    // unless the method being corrected--or the current DeclContext, if
> the
> +    // function being corrected is not a method--is a method in the same
> class
> +    // or a descendent class of the candidate's parent class.
>      if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {
> -      if (!AllowNonStaticMethods && !MD->isStatic()) {
> -        CXXMethodDecl *CurMD =
> dyn_cast_or_null<CXXMethodDecl>(CurContext);
> +      if (MemberFn || !MD->isStatic()) {
> +        CXXMethodDecl *CurMD =
> +            MemberFn
> +                ?
> dyn_cast_or_null<CXXMethodDecl>(MemberFn->getMemberDecl())
> +                : dyn_cast_or_null<CXXMethodDecl>(CurContext);
>          CXXRecordDecl *CurRD =
>              CurMD ? CurMD->getParent()->getCanonicalDecl() : 0;
>          CXXRecordDecl *RD = MD->getParent()->getCanonicalDecl();
>
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=205653&r1=205652&r2=205653&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Fri Apr  4 17:16:30 2014
> @@ -10383,7 +10383,8 @@ BuildRecoveryCallExpr(Sema &SemaRef, Sco
>    LookupResult R(SemaRef, ULE->getName(), ULE->getNameLoc(),
>                   Sema::LookupOrdinaryName);
>    FunctionCallFilterCCC Validator(SemaRef, Args.size(),
> -                                  ExplicitTemplateArgs != 0, false);
> +                                  ExplicitTemplateArgs != 0,
> +                                  dyn_cast<MemberExpr>(Fn));
>    NoTypoCorrectionCCC RejectAll;
>    CorrectionCandidateCallback *CCC = AllowTypoCorrection ?
>        (CorrectionCandidateCallback*)&Validator :
>
> Modified: cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp?rev=205653&r1=205652&r2=205653&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp (original)
> +++ cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp Fri Apr  4 17:16:30 2014
> @@ -242,6 +242,28 @@ void func() {
>    };
>    bar();  // expected-error-re {{use of undeclared identifier 'bar'{{$}}}}
>  }
> +
> +class Thread {
> + public:
> +  void Start();
> +  static void Stop();  // expected-note {{'Thread::Stop' declared here}}
> +};
> +
> +class Manager {
> + public:
> +  void Start(int);  // expected-note {{'Start' declared here}}
> +  void Stop(int);  // expected-note {{'Stop' declared here}}
> +};
> +
> +void test(Manager *m) {
> +  // Don't suggest Thread::Start as a correction just because it has the
> same
> +  // (unqualified) name and accepts the right number of args; this is a
> method
> +  // call on an object in an unrelated class.
> +  m->Start();  // expected-error-re {{too few arguments to function call,
> expected 1, have 0{{$}}}}
> +  m->Stop();  // expected-error-re {{too few arguments to function call,
> expected 1, have 0{{$}}}}
> +  Stop();  // expected-error {{use of undeclared identifier 'Stop'; did
> you mean 'Thread::Stop'?}}
> +}
> +
>  }
>
>  namespace std {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140413/74da0b29/attachment.html>


More information about the cfe-commits mailing list