r205653 - Try harder about not suggesting methods as corrections when they
Kaelyn Takata
rikka at google.com
Mon Apr 14 14:19:11 PDT 2014
On Sun, Apr 13, 2014 at 3:57 PM, Richard Smith <richard at metafoo.co.uk>wrote:
> 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.
>
Because data members aren't affected by the problem this change addresses.
Typo correction can occur on member function names in two separate
contexts: when looking up the member reference, and when handling the
function call itself. The typo correction that occurred when looking up the
member reference (e.g. because there's no member named "Foo" but there is
one named "foo") already had the right logic, and works for both function
and data members. This change fixed the second context, when typo
correction occurred during the handling of the actual function call (well
after the member reference was resolved), e.g. when the member function was
called with the wrong number of arguments.
>
>> 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/20140414/0d761c59/attachment.html>
More information about the cfe-commits
mailing list