[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
nik added a comment. Huch, seems already submitted. Ignore :> Repository: rL LLVM https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
nik added a comment. Herald added a subscriber: llvm-commits. Ping Repository: rL LLVM https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added inline comments. Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6365 cast(FD)->getParent(), ObjectType, - ObjectClassification, Args.slice(1), CandidateSet, + ObjectClassification, FunctionArgs, CandidateSet, SuppressUserConversions, PartialOverloading); cameron314 wrote: > yvvan wrote: > > cameron314 wrote: > > > This breaks normal (non-static) method overload resolution, since the > > > `this` argument is now passed to `AddMethodCandidate` which is not > > > expecting it. It always thinks too many arguments are passed to methods > > > with no parameters; most other calls to `AddMethodCandidate` in > > > SemaOverload.cpp don't pass the implicit `this`. > > This code is correct. It is sliced at line 6361 - only in the case when the > > args size is greater than 0. > Hmm, you're right, I didn't see that. That line was missing after a rebase on > our side, my fault for not properly cross-referencing the diffs. Sorry for > the false alarm. > > But shouldn't the slice only be done now when `Args.size() > 0 && (!Args[0] > || (FirstArgumentIsBase && isa(FD) && > !isa(FD)))`? there's already a check for this case that it's a CXXMethodDecl, it's not static. That means that we always need to slice (except those rare cases when we don't have any args, and in fact I don't know why these cases exist) Repository: rL LLVM https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
cameron314 added inline comments.
Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6365
cast(FD)->getParent(), ObjectType,
- ObjectClassification, Args.slice(1), CandidateSet,
+ ObjectClassification, FunctionArgs, CandidateSet,
SuppressUserConversions, PartialOverloading);
yvvan wrote:
> cameron314 wrote:
> > This breaks normal (non-static) method overload resolution, since the
> > `this` argument is now passed to `AddMethodCandidate` which is not
> > expecting it. It always thinks too many arguments are passed to methods
> > with no parameters; most other calls to `AddMethodCandidate` in
> > SemaOverload.cpp don't pass the implicit `this`.
> This code is correct. It is sliced at line 6361 - only in the case when the
> args size is greater than 0.
Hmm, you're right, I didn't see that. That line was missing after a rebase on
our side, my fault for not properly cross-referencing the diffs. Sorry for the
false alarm.
But shouldn't the slice only be done now when `Args.size() > 0 && (!Args[0] ||
(FirstArgumentIsBase && isa(FD) &&
!isa(FD)))`?
Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6396
} else {
AddTemplateOverloadCandidate(FunTmpl, F.getPair(),
ExplicitTemplateArgs, Args,
yvvan wrote:
> cameron314 wrote:
> > The same slice that was added above needs to be done here for templated
> > static methods that are accessed via a non-static object.
> It might be the case. You can add that :)
> I did not check template cases in details.
It is the case, I have parameter completion tests that fail without this :-)
I'll commit in a few days.
Repository:
rL LLVM
https://reviews.llvm.org/D36390
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added inline comments.
Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6365
cast(FD)->getParent(), ObjectType,
- ObjectClassification, Args.slice(1), CandidateSet,
+ ObjectClassification, FunctionArgs, CandidateSet,
SuppressUserConversions, PartialOverloading);
cameron314 wrote:
> This breaks normal (non-static) method overload resolution, since the `this`
> argument is now passed to `AddMethodCandidate` which is not expecting it. It
> always thinks too many arguments are passed to methods with no parameters;
> most other calls to `AddMethodCandidate` in SemaOverload.cpp don't pass the
> implicit `this`.
This code is correct. It is sliced at line 6361 - only in the case when the
args size is greater than 0.
Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6396
} else {
AddTemplateOverloadCandidate(FunTmpl, F.getPair(),
ExplicitTemplateArgs, Args,
cameron314 wrote:
> The same slice that was added above needs to be done here for templated
> static methods that are accessed via a non-static object.
It might be the case. You can add that :)
I did not check template cases in details.
Repository:
rL LLVM
https://reviews.llvm.org/D36390
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
cameron314 added inline comments.
Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6396
} else {
AddTemplateOverloadCandidate(FunTmpl, F.getPair(),
ExplicitTemplateArgs, Args,
The same slice that was added above needs to be done here for templated static
methods that are accessed via a non-static object.
Repository:
rL LLVM
https://reviews.llvm.org/D36390
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
cameron314 added inline comments. Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6365 cast(FD)->getParent(), ObjectType, - ObjectClassification, Args.slice(1), CandidateSet, + ObjectClassification, FunctionArgs, CandidateSet, SuppressUserConversions, PartialOverloading); This breaks normal (non-static) method overload resolution, since the `this` argument is now passed to `AddMethodCandidate` which is not expecting it. It always thinks too many arguments are passed to methods with no parameters; most other calls to `AddMethodCandidate` in SemaOverload.cpp don't pass the implicit `this`. Repository: rL LLVM https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316646: Fix overloaded static functions in SemaCodeComplete
(authored by d0k).
Changed prior to commit:
https://reviews.llvm.org/D36390?vs=111647=120374#toc
Repository:
rL LLVM
https://reviews.llvm.org/D36390
Files:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/Index/complete-call.cpp
Index: cfe/trunk/test/Index/complete-call.cpp
===
--- cfe/trunk/test/Index/complete-call.cpp
+++ cfe/trunk/test/Index/complete-call.cpp
@@ -94,6 +94,24 @@
s.foo_7(42,);
}
+struct Bar {
+ static void foo_1();
+ void foo_1(float);
+ static void foo_1(int);
+};
+
+void test() {
+ Bar::foo_1();
+ Bar b;
+ b.foo_1();
+}
+
+struct Bar2 : public Bar {
+ Bar2() {
+Bar::foo_1();
+ }
+};
+
// RUN: c-index-test -code-completion-at=%s:47:9 %s | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
// CHECK-CC1: Completion contexts:
@@ -803,3 +821,46 @@
// CHECK-CC59-NEXT: Class name
// CHECK-CC59-NEXT: Nested name specifier
// CHECK-CC59-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:104:14 %s | FileCheck -check-prefix=CHECK-CC60 %s
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter float}{RightParen )} (1)
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC60: Completion contexts:
+// CHECK-CC60-NEXT: Any type
+// CHECK-CC60-NEXT: Any value
+// CHECK-CC60-NEXT: Enum tag
+// CHECK-CC60-NEXT: Union tag
+// CHECK-CC60-NEXT: Struct tag
+// CHECK-CC60-NEXT: Class name
+// CHECK-CC60-NEXT: Nested name specifier
+// CHECK-CC60-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:106:11 %s | FileCheck -check-prefix=CHECK-CC61 %s
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter float}{RightParen )} (1)
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC61: Completion contexts:
+// CHECK-CC61-NEXT: Any type
+// CHECK-CC61-NEXT: Any value
+// CHECK-CC61-NEXT: Enum tag
+// CHECK-CC61-NEXT: Union tag
+// CHECK-CC61-NEXT: Struct tag
+// CHECK-CC61-NEXT: Class name
+// CHECK-CC61-NEXT: Nested name specifier
+// CHECK-CC61-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:111:16 %s | FileCheck -check-prefix=CHECK-CC62 %s
+// CHECK-CC62: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
+// CHECK-CC62: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter float}{RightParen )} (1)
+// CHECK-CC62: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC62: Completion contexts:
+// CHECK-CC62-NEXT: Any type
+// CHECK-CC62-NEXT: Any value
+// CHECK-CC62-NEXT: Enum tag
+// CHECK-CC62-NEXT: Union tag
+// CHECK-CC62-NEXT: Struct tag
+// CHECK-CC62-NEXT: Class name
+// CHECK-CC62-NEXT: Nested name specifier
+// CHECK-CC62-NEXT: Objective-C interface
+
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -6343,24 +6343,36 @@
OverloadCandidateSet& CandidateSet,
TemplateArgumentListInfo *ExplicitTemplateArgs,
bool SuppressUserConversions,
- bool PartialOverloading) {
+ bool PartialOverloading,
+ bool FirstArgumentIsBase) {
for (UnresolvedSetIterator F = Fns.begin(), E = Fns.end(); F != E; ++F) {
NamedDecl *D = F.getDecl()->getUnderlyingDecl();
if (FunctionDecl *FD = dyn_cast(D)) {
+ ArrayRef FunctionArgs = Args;
if (isa(FD) && !cast(FD)->isStatic()) {
QualType ObjectType;
Expr::Classification ObjectClassification;
-if (Expr *E = Args[0]) {
- // Use the explit base to restrict the lookup:
- ObjectType = E->getType();
- ObjectClassification = E->Classify(Context);
-} // .. else there is an implit base.
+if (Args.size() > 0) {
+ if (Expr *E = Args[0]) {
+// Use the explit base to restrict the lookup:
+ObjectType = E->getType();
+ObjectClassification = E->Classify(Context);
+ } // .. else there is an implit base.
+
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added a comment. @bkramer Not yet, it would be good to have one though :) https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. This looks good. Sorry for the long wait, do you have commit access? https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added a comment. one more ping... https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added a comment. ping 3 https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added a comment. one more ping :) https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added a comment. ping https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan updated this revision to Diff 111647.
yvvan marked 5 inline comments as done.
yvvan added a comment.
Review comments + solved https://bugs.llvm.org/show_bug.cgi?id=34207 issue.
I've also added the extra test for that issue
https://reviews.llvm.org/D36390
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaCodeComplete.cpp
lib/Sema/SemaOverload.cpp
test/Index/complete-call.cpp
Index: test/Index/complete-call.cpp
===
--- test/Index/complete-call.cpp
+++ test/Index/complete-call.cpp
@@ -94,6 +94,24 @@
s.foo_7(42,);
}
+struct Bar {
+ static void foo_1();
+ void foo_1(float);
+ static void foo_1(int);
+};
+
+void test() {
+ Bar::foo_1();
+ Bar b;
+ b.foo_1();
+}
+
+struct Bar2 : public Bar {
+ Bar2() {
+Bar::foo_1();
+ }
+};
+
// RUN: c-index-test -code-completion-at=%s:47:9 %s | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
// CHECK-CC1: Completion contexts:
@@ -803,3 +821,46 @@
// CHECK-CC59-NEXT: Class name
// CHECK-CC59-NEXT: Nested name specifier
// CHECK-CC59-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:104:14 %s | FileCheck -check-prefix=CHECK-CC60 %s
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter float}{RightParen )} (1)
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC60: Completion contexts:
+// CHECK-CC60-NEXT: Any type
+// CHECK-CC60-NEXT: Any value
+// CHECK-CC60-NEXT: Enum tag
+// CHECK-CC60-NEXT: Union tag
+// CHECK-CC60-NEXT: Struct tag
+// CHECK-CC60-NEXT: Class name
+// CHECK-CC60-NEXT: Nested name specifier
+// CHECK-CC60-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:106:11 %s | FileCheck -check-prefix=CHECK-CC61 %s
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter float}{RightParen )} (1)
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC61: Completion contexts:
+// CHECK-CC61-NEXT: Any type
+// CHECK-CC61-NEXT: Any value
+// CHECK-CC61-NEXT: Enum tag
+// CHECK-CC61-NEXT: Union tag
+// CHECK-CC61-NEXT: Struct tag
+// CHECK-CC61-NEXT: Class name
+// CHECK-CC61-NEXT: Nested name specifier
+// CHECK-CC61-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:111:16 %s | FileCheck -check-prefix=CHECK-CC62 %s
+// CHECK-CC62: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
+// CHECK-CC62: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter float}{RightParen )} (1)
+// CHECK-CC62: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC62: Completion contexts:
+// CHECK-CC62-NEXT: Any type
+// CHECK-CC62-NEXT: Any value
+// CHECK-CC62-NEXT: Enum tag
+// CHECK-CC62-NEXT: Union tag
+// CHECK-CC62-NEXT: Struct tag
+// CHECK-CC62-NEXT: Class name
+// CHECK-CC62-NEXT: Nested name specifier
+// CHECK-CC62-NEXT: Objective-C interface
+
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -6317,24 +6317,36 @@
OverloadCandidateSet& CandidateSet,
TemplateArgumentListInfo *ExplicitTemplateArgs,
bool SuppressUserConversions,
- bool PartialOverloading) {
+ bool PartialOverloading,
+ bool FirstArgumentIsBase) {
for (UnresolvedSetIterator F = Fns.begin(), E = Fns.end(); F != E; ++F) {
NamedDecl *D = F.getDecl()->getUnderlyingDecl();
if (FunctionDecl *FD = dyn_cast(D)) {
+ ArrayRef FunctionArgs = Args;
if (isa(FD) && !cast(FD)->isStatic()) {
QualType ObjectType;
Expr::Classification ObjectClassification;
-if (Expr *E = Args[0]) {
- // Use the explit base to restrict the lookup:
- ObjectType = E->getType();
- ObjectClassification = E->Classify(Context);
-} // .. else there is an implit base.
+if (Args.size() > 0) {
+ if (Expr *E = Args[0]) {
+// Use the explit base to restrict the lookup:
+ObjectType = E->getType();
+ObjectClassification = E->Classify(Context);
+ } // .. else there is an implit base.
+ FunctionArgs = Args.slice(1);
+}
AddMethodCandidate(cast(FD), F.getPair(),
cast(FD)->getParent(), ObjectType,
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added inline comments.
Comment at: lib/Sema/SemaOverload.cpp:6342
+&& !isa(FD)) {
+ Args = Args.slice(1);
+}
nik wrote:
> bkramer wrote:
> > assert that FD is a static method.
> Just stumbled here because I was looking into
> https://bugs.llvm.org/show_bug.cgi?id=34207
> Some observations
> * Slicing the first argument unconditionally in this branch (without the if
> at 6340) fixes the issue.
> * The current revision/version of this change does not fix the issue.
>
> In case the crash is adapted with this patch, fine. Otherwise I'll wait until
> this one got in and look again at it.
"Slicing the first argument unconditionally in this branch" is not valid
because constructors are affected (at least them) where this argument is needed
https://reviews.llvm.org/D36390
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
nik added inline comments.
Comment at: lib/Sema/SemaOverload.cpp:6342
+&& !isa(FD)) {
+ Args = Args.slice(1);
+}
bkramer wrote:
> assert that FD is a static method.
Just stumbled here because I was looking into
https://bugs.llvm.org/show_bug.cgi?id=34207
Some observations
* Slicing the first argument unconditionally in this branch (without the if at
6340) fixes the issue.
* The current revision/version of this change does not fix the issue.
In case the crash is adapted with this patch, fine. Otherwise I'll wait until
this one got in and look again at it.
https://reviews.llvm.org/D36390
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
bkramer added inline comments.
Comment at: include/clang/Sema/Sema.h:2681
+ bool PartialOverloading = false,
+ bool ExtraFirstArgument = false);
void AddMethodCandidate(DeclAccessPair FoundDecl,
I'd prefer something like "FirstArgumentIsBase"
Comment at: lib/Sema/SemaOverload.cpp:5871
// is irrelevant.
+
AddMethodCandidate(Method, FoundDecl, Method->getParent(), QualType(),
Unnecessary whitespace change.
Comment at: lib/Sema/SemaOverload.cpp:6339
} else {
+// Slice the first argument when we access static method as non-static
+if (Args.size() > 0 && ExtraFirstArgument && isa(FD)
Add a comment that the first argument is the base.
Comment at: lib/Sema/SemaOverload.cpp:6341
+if (Args.size() > 0 && ExtraFirstArgument && isa(FD)
+&& !isa(FD)) {
+ Args = Args.slice(1);
clang-format
Comment at: lib/Sema/SemaOverload.cpp:6342
+&& !isa(FD)) {
+ Args = Args.slice(1);
+}
assert that FD is a static method.
https://reviews.llvm.org/D36390
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added a comment. Please check that one https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan added a comment. I've just found a regression in my change. in case I have std::string(/*complete here*/) I need to investigate that case because I thought it's covered by !isa(FD) ... https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan updated this revision to Diff 109980.
yvvan added a comment.
Yes, I missed to include one file in this diff where it's used
https://reviews.llvm.org/D36390
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaCodeComplete.cpp
lib/Sema/SemaOverload.cpp
test/Index/complete-call.cpp
Index: test/Index/complete-call.cpp
===
--- test/Index/complete-call.cpp
+++ test/Index/complete-call.cpp
@@ -94,6 +94,17 @@
s.foo_7(42,);
}
+struct Bar {
+ static void foo_1();
+ static void foo_1(int);
+};
+
+void test() {
+ Bar::foo_1();
+ Bar b;
+ b.foo_1();
+}
+
// RUN: c-index-test -code-completion-at=%s:47:9 %s | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
// CHECK-CC1: Completion contexts:
@@ -803,3 +814,29 @@
// CHECK-CC59-NEXT: Class name
// CHECK-CC59-NEXT: Nested name specifier
// CHECK-CC59-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:103:14 %s | FileCheck -check-prefix=CHECK-CC60 %s
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC60: Completion contexts:
+// CHECK-CC60-NEXT: Any type
+// CHECK-CC60-NEXT: Any value
+// CHECK-CC60-NEXT: Enum tag
+// CHECK-CC60-NEXT: Union tag
+// CHECK-CC60-NEXT: Struct tag
+// CHECK-CC60-NEXT: Class name
+// CHECK-CC60-NEXT: Nested name specifier
+// CHECK-CC60-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:105:11 %s | FileCheck -check-prefix=CHECK-CC61 %s
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{RightParen )} (1)
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen (}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC61: Completion contexts:
+// CHECK-CC61-NEXT: Any type
+// CHECK-CC61-NEXT: Any value
+// CHECK-CC61-NEXT: Enum tag
+// CHECK-CC61-NEXT: Union tag
+// CHECK-CC61-NEXT: Struct tag
+// CHECK-CC61-NEXT: Class name
+// CHECK-CC61-NEXT: Nested name specifier
+// CHECK-CC61-NEXT: Objective-C interface
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5868,6 +5868,7 @@
// function, e.g., X::f(). We use an empty type for the implied
// object argument (C++ [over.call.func]p3), and the acting context
// is irrelevant.
+
AddMethodCandidate(Method, FoundDecl, Method->getParent(), QualType(),
Expr::Classification::makeSimpleLValue(), Args,
CandidateSet, SuppressUserConversions,
@@ -6317,7 +6318,8 @@
OverloadCandidateSet& CandidateSet,
TemplateArgumentListInfo *ExplicitTemplateArgs,
bool SuppressUserConversions,
- bool PartialOverloading) {
+ bool PartialOverloading,
+ bool ExtraFirstArgument) {
for (UnresolvedSetIterator F = Fns.begin(), E = Fns.end(); F != E; ++F) {
NamedDecl *D = F.getDecl()->getUnderlyingDecl();
if (FunctionDecl *FD = dyn_cast(D)) {
@@ -6334,6 +6336,11 @@
ObjectClassification, Args.slice(1), CandidateSet,
SuppressUserConversions, PartialOverloading);
} else {
+// Slice the first argument when we access static method as non-static
+if (Args.size() > 0 && ExtraFirstArgument && isa(FD)
+&& !isa(FD)) {
+ Args = Args.slice(1);
+}
AddOverloadCandidate(FD, F.getPair(), Args, CandidateSet,
SuppressUserConversions, PartialOverloading);
}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4379,9 +4379,11 @@
ArgExprs.append(Args.begin(), Args.end());
UnresolvedSet<8> Decls;
Decls.append(UME->decls_begin(), UME->decls_end());
+const bool extraArgument = !UME->isImplicitAccess() && UME->getBase();
AddFunctionCandidates(Decls, ArgExprs, CandidateSet, TemplateArgs,
/*SuppressUsedConversions=*/false,
- /*PartialOverloading=*/true);
+ /*PartialOverloading=*/true,
+ extraArgument);
} else {
FunctionDecl *FD = nullptr;
if (auto MCE = dyn_cast(NakedFn))
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2677,7 +2677,8 @@
OverloadCandidateSet ,
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
bkramer added inline comments. Comment at: include/clang/Sema/Sema.h:2681 + bool PartialOverloading = false, + bool ExtraFirstArgument = false); void AddMethodCandidate(DeclAccessPair FoundDecl, Shouldn't this be called with the new argument somehere? Otherwise it'll always be false. https://reviews.llvm.org/D36390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete
yvvan created this revision.
https://bugs.llvm.org/show_bug.cgi?id=33904
Happens when static function is accessed via the class variable. That leads to
incorrect overloads number because the variable is considered as the first
argument.
struct Bar {
static void foo(); static void foo(int);
};
int main() {
Bar b;
b.foo(/*complete here*/); // did not work before
Bar::foo(/*complete here*/); // worked fine
}
https://reviews.llvm.org/D36390
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaOverload.cpp
test/Index/complete-call.cpp
Index: test/Index/complete-call.cpp
===
--- test/Index/complete-call.cpp
+++ test/Index/complete-call.cpp
@@ -94,6 +94,17 @@
s.foo_7(42,);
}
+struct Bar {
+ static void foo_1();
+ static void foo_1(int);
+};
+
+void test() {
+ Bar::foo_1();
+ Bar b;
+ b.foo_1();
+}
+
// RUN: c-index-test -code-completion-at=%s:47:9 %s | FileCheck
-check-prefix=CHECK-CC1 %s
// CHECK-CC1: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen
(}{RightParen )} (1)
// CHECK-CC1: Completion contexts:
@@ -803,3 +814,29 @@
// CHECK-CC59-NEXT: Class name
// CHECK-CC59-NEXT: Nested name specifier
// CHECK-CC59-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:103:14 %s | FileCheck
-check-prefix=CHECK-CC60 %s
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen
(}{RightParen )} (1)
+// CHECK-CC60: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen
(}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC60: Completion contexts:
+// CHECK-CC60-NEXT: Any type
+// CHECK-CC60-NEXT: Any value
+// CHECK-CC60-NEXT: Enum tag
+// CHECK-CC60-NEXT: Union tag
+// CHECK-CC60-NEXT: Struct tag
+// CHECK-CC60-NEXT: Class name
+// CHECK-CC60-NEXT: Nested name specifier
+// CHECK-CC60-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:105:11 %s | FileCheck
-check-prefix=CHECK-CC61 %s
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen
(}{RightParen )} (1)
+// CHECK-CC61: OverloadCandidate:{ResultType void}{Text foo_1}{LeftParen
(}{CurrentParameter int}{RightParen )} (1)
+// CHECK-CC61: Completion contexts:
+// CHECK-CC61-NEXT: Any type
+// CHECK-CC61-NEXT: Any value
+// CHECK-CC61-NEXT: Enum tag
+// CHECK-CC61-NEXT: Union tag
+// CHECK-CC61-NEXT: Struct tag
+// CHECK-CC61-NEXT: Class name
+// CHECK-CC61-NEXT: Nested name specifier
+// CHECK-CC61-NEXT: Objective-C interface
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5868,6 +5868,7 @@
// function, e.g., X::f(). We use an empty type for the implied
// object argument (C++ [over.call.func]p3), and the acting context
// is irrelevant.
+
AddMethodCandidate(Method, FoundDecl, Method->getParent(), QualType(),
Expr::Classification::makeSimpleLValue(), Args,
CandidateSet, SuppressUserConversions,
@@ -6317,7 +6318,8 @@
OverloadCandidateSet& CandidateSet,
TemplateArgumentListInfo
*ExplicitTemplateArgs,
bool SuppressUserConversions,
- bool PartialOverloading) {
+ bool PartialOverloading,
+ bool ExtraFirstArgument) {
for (UnresolvedSetIterator F = Fns.begin(), E = Fns.end(); F != E; ++F) {
NamedDecl *D = F.getDecl()->getUnderlyingDecl();
if (FunctionDecl *FD = dyn_cast(D)) {
@@ -6334,6 +6336,11 @@
ObjectClassification, Args.slice(1), CandidateSet,
SuppressUserConversions, PartialOverloading);
} else {
+// Slice the first argument when we access static method as non-static
+if (Args.size() > 0 && ExtraFirstArgument && isa(FD)
+&& !isa(FD)) {
+ Args = Args.slice(1);
+}
AddOverloadCandidate(FD, F.getPair(), Args, CandidateSet,
SuppressUserConversions, PartialOverloading);
}
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2677,7 +2677,8 @@
OverloadCandidateSet ,
TemplateArgumentListInfo *ExplicitTemplateArgs = nullptr,
bool SuppressUserConversions = false,
- bool PartialOverloading = false);
+ bool PartialOverloading = false,
+ bool ExtraFirstArgument = false);
void AddMethodCandidate(DeclAccessPair FoundDecl,
QualType ObjectType,
Expr::Classification ObjectClassification,
Index: test/Index/complete-call.cpp
