<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Apr 13, 2014 at 3:57 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=richard@metafoo.co.uk&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
On Fri, Apr 4, 2014 at 3:16 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=rikka@google.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">rikka@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: rikka<br>
Date: Fri Apr 4 17:16:30 2014<br>
New Revision: 205653<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=205653&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=205653&view=rev</a><br>
Log:<br>
Try harder about not suggesting methods as corrections when they<br>
obviously won't work. Specifically, don't suggest methods (static or<br>
not) from unrelated classes when the expression is a method call<br>
through a specific object.<br></blockquote><div><br></div><div>Why only member functions? This seems like the right logic for data members too.</div></div></div></div></blockquote><div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Modified:<br>
cfe/trunk/include/clang/Sema/TypoCorrection.h<br>
cfe/trunk/lib/Sema/SemaExpr.cpp<br>
cfe/trunk/lib/Sema/SemaLookup.cpp<br>
cfe/trunk/lib/Sema/SemaOverload.cpp<br>
cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Sema/TypoCorrection.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/TypoCorrection.h?rev=205653&r1=205652&r2=205653&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/TypoCorrection.h?rev=205653&r1=205652&r2=205653&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Sema/TypoCorrection.h (original)<br>
+++ cfe/trunk/include/clang/Sema/TypoCorrection.h Fri Apr 4 17:16:30 2014<br>
@@ -306,15 +306,15 @@ class FunctionCallFilterCCC : public Cor<br>
public:<br>
FunctionCallFilterCCC(Sema &SemaRef, unsigned NumArgs,<br>
bool HasExplicitTemplateArgs,<br>
- bool AllowNonStaticMethods = true);<br>
+ MemberExpr *ME = 0);<br>
<br>
bool ValidateCandidate(const TypoCorrection &candidate) override;<br>
<br>
private:<br>
unsigned NumArgs;<br>
bool HasExplicitTemplateArgs;<br>
- bool AllowNonStaticMethods;<br>
DeclContext *CurContext;<br>
+ MemberExpr *MemberFn;<br>
};<br>
<br>
// @brief Callback class that effectively disabled typo correction<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=205653&r1=205652&r2=205653&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=205653&r1=205652&r2=205653&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Apr 4 17:16:30 2014<br>
@@ -3974,8 +3974,8 @@ namespace {<br>
class FunctionCallCCC : public FunctionCallFilterCCC {<br>
public:<br>
FunctionCallCCC(Sema &SemaRef, const IdentifierInfo *FuncName,<br>
- unsigned NumArgs, bool HasExplicitTemplateArgs)<br>
- : FunctionCallFilterCCC(SemaRef, NumArgs, HasExplicitTemplateArgs),<br>
+ unsigned NumArgs, MemberExpr *ME)<br>
+ : FunctionCallFilterCCC(SemaRef, NumArgs, false, ME),<br>
FunctionName(FuncName) {}<br>
<br>
bool ValidateCandidate(const TypoCorrection &candidate) override {<br>
@@ -3992,17 +3992,20 @@ private:<br>
};<br>
}<br>
<br>
-static TypoCorrection TryTypoCorrectionForCall(Sema &S,<br>
- DeclarationNameInfo FuncName,<br>
+static TypoCorrection TryTypoCorrectionForCall(Sema &S, Expr *Fn,<br>
+ FunctionDecl *FDecl,<br>
ArrayRef<Expr *> Args) {<br>
- FunctionCallCCC CCC(S, FuncName.getName().getAsIdentifierInfo(),<br>
- Args.size(), false);<br>
- if (TypoCorrection Corrected =<br>
- S.CorrectTypo(FuncName, Sema::LookupOrdinaryName,<br>
- S.getScopeForContext(S.CurContext), NULL, CCC)) {<br>
+ MemberExpr *ME = dyn_cast<MemberExpr>(Fn);<br>
+ DeclarationName FuncName = FDecl->getDeclName();<br>
+ SourceLocation NameLoc = ME ? ME->getMemberLoc() : Fn->getLocStart();<br>
+ FunctionCallCCC CCC(S, FuncName.getAsIdentifierInfo(), Args.size(), ME);<br>
+<br>
+ if (TypoCorrection Corrected = S.CorrectTypo(<br>
+ DeclarationNameInfo(FuncName, NameLoc), Sema::LookupOrdinaryName,<br>
+ S.getScopeForContext(S.CurContext), NULL, CCC)) {<br>
if (NamedDecl *ND = Corrected.getCorrectionDecl()) {<br>
if (Corrected.isOverloaded()) {<br>
- OverloadCandidateSet OCS(FuncName.getLoc());<br>
+ OverloadCandidateSet OCS(NameLoc);<br>
OverloadCandidateSet::iterator Best;<br>
for (TypoCorrection::decl_iterator CD = Corrected.begin(),<br>
CDEnd = Corrected.end();<br>
@@ -4011,7 +4014,7 @@ static TypoCorrection TryTypoCorrectionF<br>
S.AddOverloadCandidate(FD, DeclAccessPair::make(FD, AS_none), Args,<br>
OCS);<br>
}<br>
- switch (OCS.BestViableFunction(S, FuncName.getLoc(), Best)) {<br>
+ switch (OCS.BestViableFunction(S, NameLoc, Best)) {<br>
case OR_Success:<br>
ND = Best->Function;<br>
Corrected.setCorrectionDecl(ND);<br>
@@ -4062,13 +4065,8 @@ Sema::ConvertArgumentsForCall(CallExpr *<br>
// arguments for the remaining parameters), don't make the call.<br>
if (Args.size() < NumParams) {<br>
if (Args.size() < MinArgs) {<br>
- MemberExpr *ME = dyn_cast<MemberExpr>(Fn);<br>
TypoCorrection TC;<br>
- if (FDecl && (TC = TryTypoCorrectionForCall(<br>
- *this, DeclarationNameInfo(FDecl->getDeclName(),<br>
- (ME ? ME->getMemberLoc()<br>
- : Fn->getLocStart())),<br>
- Args))) {<br>
+ if (FDecl && (TC = TryTypoCorrectionForCall(*this, Fn, FDecl, Args))) {<br>
unsigned diag_id =<br>
MinArgs == NumParams && !Proto->isVariadic()<br>
? diag::err_typecheck_call_too_few_args_suggest<br>
@@ -4103,13 +4101,8 @@ Sema::ConvertArgumentsForCall(CallExpr *<br>
// them.<br>
if (Args.size() > NumParams) {<br>
if (!Proto->isVariadic()) {<br>
- MemberExpr *ME = dyn_cast<MemberExpr>(Fn);<br>
TypoCorrection TC;<br>
- if (FDecl && (TC = TryTypoCorrectionForCall(<br>
- *this, DeclarationNameInfo(FDecl->getDeclName(),<br>
- (ME ? ME->getMemberLoc()<br>
- : Fn->getLocStart())),<br>
- Args))) {<br>
+ if (FDecl && (TC = TryTypoCorrectionForCall(*this, Fn, FDecl, Args))) {<br>
unsigned diag_id =<br>
MinArgs == NumParams && !Proto->isVariadic()<br>
? diag::err_typecheck_call_too_many_args_suggest<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=205653&r1=205652&r2=205653&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=205653&r1=205652&r2=205653&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Apr 4 17:16:30 2014<br>
@@ -4511,10 +4511,9 @@ bool CorrectionCandidateCallback::Valida<br>
<br>
FunctionCallFilterCCC::FunctionCallFilterCCC(Sema &SemaRef, unsigned NumArgs,<br>
bool HasExplicitTemplateArgs,<br>
- bool AllowNonStaticMethods)<br>
+ MemberExpr *ME)<br>
: NumArgs(NumArgs), HasExplicitTemplateArgs(HasExplicitTemplateArgs),<br>
- AllowNonStaticMethods(AllowNonStaticMethods),<br>
- CurContext(SemaRef.CurContext) {<br>
+ CurContext(SemaRef.CurContext), MemberFn(ME) {<br>
WantTypeSpecifiers = SemaRef.getLangOpts().CPlusPlus;<br>
WantRemainingKeywords = false;<br>
}<br>
@@ -4550,13 +4549,16 @@ bool FunctionCallFilterCCC::ValidateCand<br>
FD->getMinRequiredArguments() <= NumArgs))<br>
continue;<br>
<br>
- // If the current candidate is a non-static C++ method and non-static<br>
- // methods are being excluded, then skip the candidate unless the current<br>
- // DeclContext is a method in the same class or a descendent class of the<br>
- // candidate's parent class.<br>
+ // If the current candidate is a non-static C++ method, skip the candidate<br>
+ // unless the method being corrected--or the current DeclContext, if the<br>
+ // function being corrected is not a method--is a method in the same class<br>
+ // or a descendent class of the candidate's parent class.<br>
if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {<br>
- if (!AllowNonStaticMethods && !MD->isStatic()) {<br>
- CXXMethodDecl *CurMD = dyn_cast_or_null<CXXMethodDecl>(CurContext);<br>
+ if (MemberFn || !MD->isStatic()) {<br>
+ CXXMethodDecl *CurMD =<br>
+ MemberFn<br>
+ ? dyn_cast_or_null<CXXMethodDecl>(MemberFn->getMemberDecl())<br>
+ : dyn_cast_or_null<CXXMethodDecl>(CurContext);<br>
CXXRecordDecl *CurRD =<br>
CurMD ? CurMD->getParent()->getCanonicalDecl() : 0;<br>
CXXRecordDecl *RD = MD->getParent()->getCanonicalDecl();<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=205653&r1=205652&r2=205653&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=205653&r1=205652&r2=205653&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Fri Apr 4 17:16:30 2014<br>
@@ -10383,7 +10383,8 @@ BuildRecoveryCallExpr(Sema &SemaRef, Sco<br>
LookupResult R(SemaRef, ULE->getName(), ULE->getNameLoc(),<br>
Sema::LookupOrdinaryName);<br>
FunctionCallFilterCCC Validator(SemaRef, Args.size(),<br>
- ExplicitTemplateArgs != 0, false);<br>
+ ExplicitTemplateArgs != 0,<br>
+ dyn_cast<MemberExpr>(Fn));<br>
NoTypoCorrectionCCC RejectAll;<br>
CorrectionCandidateCallback *CCC = AllowTypoCorrection ?<br>
(CorrectionCandidateCallback*)&Validator :<br>
<br>
Modified: cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp?rev=205653&r1=205652&r2=205653&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp?rev=205653&r1=205652&r2=205653&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp Fri Apr 4 17:16:30 2014<br>
@@ -242,6 +242,28 @@ void func() {<br>
};<br>
bar(); // expected-error-re {{use of undeclared identifier 'bar'{{$}}}}<br>
}<br>
+<br>
+class Thread {<br>
+ public:<br>
+ void Start();<br>
+ static void Stop(); // expected-note {{'Thread::Stop' declared here}}<br>
+};<br>
+<br>
+class Manager {<br>
+ public:<br>
+ void Start(int); // expected-note {{'Start' declared here}}<br>
+ void Stop(int); // expected-note {{'Stop' declared here}}<br>
+};<br>
+<br>
+void test(Manager *m) {<br>
+ // Don't suggest Thread::Start as a correction just because it has the same<br>
+ // (unqualified) name and accepts the right number of args; this is a method<br>
+ // call on an object in an unrelated class.<br>
+ m->Start(); // expected-error-re {{too few arguments to function call, expected 1, have 0{{$}}}}<br>
+ m->Stop(); // expected-error-re {{too few arguments to function call, expected 1, have 0{{$}}}}<br>
+ Stop(); // expected-error {{use of undeclared identifier 'Stop'; did you mean 'Thread::Stop'?}}<br>
+}<br>
+<br>
}<br>
<br>
namespace std {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=cfe-commits@cs.uiuc.edu&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>
</blockquote></div><br></div></div>