[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336255: [SemaCodeComplete] Make sure visited contexts are
passed to completion results… (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48917?vs=154077&id=154080#toc
Repository:
rC Clang
https://reviews.llvm.org/D48917
Files:
lib/Sema/SemaCodeComplete.cpp
unittests/Sema/CodeCompleteTest.cpp
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3700,9 +3700,11 @@
/// type we're looking for.
void Sema::CodeCompleteExpression(Scope *S,
const CodeCompleteExpressionData &Data) {
- ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Expression);
+ ResultBuilder Results(
+ *this, CodeCompleter->getAllocator(),
+ CodeCompleter->getCodeCompletionTUInfo(),
+ CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+Data.PreferredType));
if (Data.ObjCCollection)
Results.setFilter(&ResultBuilder::IsObjCCollection);
else if (Data.IntegralConstantExpression)
@@ -3741,10 +3743,8 @@
if (CodeCompleter->includeMacros())
AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext(CodeCompletionContext::CCC_Expression,
- Data.PreferredType),
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4360,11 @@
}
Results.ExitScope();
- //We need to make sure we're setting the right context,
- //so only say we include macros if the code completer says we do
- enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
if (CodeCompleter->includeMacros()) {
AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
}
-
- HandleCodeCompleteResults(this, CodeCompleter,
-kind,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4767,9 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4788,8 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteNamespaceDecl(Scope *S) {
@@ -4893,10 +4885,9 @@
// Add any type specifiers
AddTypeSpecifierResults(getLangOpts(), Results);
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5168,8 @@
else
AddObjCTopLevelResults(Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static void AddObjCExpressionResults(ResultBuilder &Results, bool NeedAt) {
@@ -5311,9 +5301,8 @@
Results.EnterNewScope();
AddObjCVisibilityResults(getLangOpts(), Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ioeric added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, ilya-biryukov wrote: > ilya-biryukov wrote: > > ioeric wrote: > > > ilya-biryukov wrote: > > > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can > > > > we pass in the context with `PreferedType` there instead of > > > > reconstructing it later? > > > > To make sure we don't miss other things (incl. any future additions) > > > > that `ResultsBuilder` puts into the context. > > > The `PreferedType` should actually already be set in the `ResultsBuilder` > > > (line 3715). In thoery, `Results.getCompletionContext()` should work fine > > > here as well, but it would break some Index tests - it introduced some > > > inconsistency in sema scoring when running with and without result > > > caching > > > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35). > > > This is probably a bug, but I'm not sure if I'm the right person to > > > chase it :( > > What kind of inconsistencies? Maybe we should just update the CHECKS in the > > test? > After chatting offline: it seems passing in the context that has the > preferred type set into `ResultsBuilder` saves us from check failures. > The fact that `ResultsBuilder` also has `setPreferrredType`, which can be > different from the one in the `CodeCompletionContext` is still confusing, but > that's unrelated to the problem at hand. Sounds good. Thanks! Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ioeric updated this revision to Diff 154077.
ioeric added a comment.
- Addressed review comment.
Repository:
rC Clang
https://reviews.llvm.org/D48917
Files:
lib/Sema/SemaCodeComplete.cpp
unittests/Sema/CodeCompleteTest.cpp
Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -131,4 +131,15 @@
EXPECT_TRUE(VisitedNS.empty());
}
+TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {
+ auto VisitedNS = runCodeCompleteOnCode(R"cpp(
+namespace n1 {
+namespace n2 {
+ void f(^) {}
+}
+}
+ )cpp");
+ EXPECT_THAT(VisitedNS, UnorderedElementsAre("n1", "n1::n2"));
+}
+
} // namespace
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3700,9 +3700,11 @@
/// type we're looking for.
void Sema::CodeCompleteExpression(Scope *S,
const CodeCompleteExpressionData &Data) {
- ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Expression);
+ ResultBuilder Results(
+ *this, CodeCompleter->getAllocator(),
+ CodeCompleter->getCodeCompletionTUInfo(),
+ CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+Data.PreferredType));
if (Data.ObjCCollection)
Results.setFilter(&ResultBuilder::IsObjCCollection);
else if (Data.IntegralConstantExpression)
@@ -3741,10 +3743,8 @@
if (CodeCompleter->includeMacros())
AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext(CodeCompletionContext::CCC_Expression,
- Data.PreferredType),
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4360,11 @@
}
Results.ExitScope();
- //We need to make sure we're setting the right context,
- //so only say we include macros if the code completer says we do
- enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
if (CodeCompleter->includeMacros()) {
AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
}
-
- HandleCodeCompleteResults(this, CodeCompleter,
-kind,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4767,9 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4788,8 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteNamespaceDecl(Scope *S) {
@@ -4893,10 +4885,9 @@
// Add any type specifiers
AddTypeSpecifierResults(getLangOpts(), Results);
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5168,8 @@
else
AddObjCTopLevelResults(Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we > > > pass in the context with `PreferedType` there instead of reconstructing > > > it later? > > > To make sure we don't miss other things (incl. any future additions) that > > > `ResultsBuilder` puts into the context. > > The `PreferedType` should actually already be set in the `ResultsBuilder` > > (line 3715). In thoery, `Results.getCompletionContext()` should work fine > > here as well, but it would break some Index tests - it introduced some > > inconsistency in sema scoring when running with and without result caching > > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35). > > This is probably a bug, but I'm not sure if I'm the right person to chase > > it :( > What kind of inconsistencies? Maybe we should just update the CHECKS in the > test? After chatting offline: it seems passing in the context that has the preferred type set into `ResultsBuilder` saves us from check failures. The fact that `ResultsBuilder` also has `setPreferrredType`, which can be different from the one in the `CodeCompletionContext` is still confusing, but that's unrelated to the problem at hand. Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, ioeric wrote: > ilya-biryukov wrote: > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we > > pass in the context with `PreferedType` there instead of reconstructing it > > later? > > To make sure we don't miss other things (incl. any future additions) that > > `ResultsBuilder` puts into the context. > The `PreferedType` should actually already be set in the `ResultsBuilder` > (line 3715). In thoery, `Results.getCompletionContext()` should work fine > here as well, but it would break some Index tests - it introduced some > inconsistency in sema scoring when running with and without result caching > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35). > This is probably a bug, but I'm not sure if I'm the right person to chase it > :( What kind of inconsistencies? Maybe we should just update the CHECKS in the test? Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ioeric added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, ilya-biryukov wrote: > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we pass > in the context with `PreferedType` there instead of reconstructing it later? > To make sure we don't miss other things (incl. any future additions) that > `ResultsBuilder` puts into the context. The `PreferedType` should actually already be set in the `ResultsBuilder` (line 3715). In thoery, `Results.getCompletionContext()` should work fine here as well, but it would break some Index tests - it introduced some inconsistency in sema scoring when running with and without result caching (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35). This is probably a bug, but I'm not sure if I'm the right person to chase it :( Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ilya-biryukov added a comment. Generally LG, just one comment. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we pass in the context with `PreferedType` there instead of reconstructing it later? To make sure we don't miss other things (incl. any future additions) that `ResultsBuilder` puts into the context. Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D48917
Files:
lib/Sema/SemaCodeComplete.cpp
unittests/Sema/CodeCompleteTest.cpp
Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -131,4 +131,15 @@
EXPECT_TRUE(VisitedNS.empty());
}
+TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {
+ auto VisitedNS = runCodeCompleteOnCode(R"cpp(
+namespace n1 {
+namespace n2 {
+ void f(^) {}
+}
+}
+ )cpp");
+ EXPECT_THAT(VisitedNS, UnorderedElementsAre("n1", "n1::n2"));
+}
+
} // namespace
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3741,10 +3741,12 @@
if (CodeCompleter->includeMacros())
AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext(CodeCompletionContext::CCC_Expression,
- Data.PreferredType),
-Results.data(),Results.size());
+ CodeCompletionContext CC(CodeCompletionContext::CCC_Expression,
+ Data.PreferredType);
+ for (auto *Visited : Results.getCompletionContext().getVisitedContexts())
+CC.addVisitedContext(Visited);
+ HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
+Results.size());
}
void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4362,11 @@
}
Results.ExitScope();
- //We need to make sure we're setting the right context,
- //so only say we include macros if the code completer says we do
- enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
if (CodeCompleter->includeMacros()) {
AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
}
-
- HandleCodeCompleteResults(this, CodeCompleter,
-kind,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4769,9 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4790,8 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteNamespaceDecl(Scope *S) {
@@ -4893,10 +4887,9 @@
// Add any type specifiers
AddTypeSpecifierResults(getLangOpts(), Results);
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5170,8 @@
else
AddObjCTopLevelResults(Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static void AddObjCExpressionResults(ResultBuilder &Results, bool NeedAt) {
@@ -5311,9 +5303,8 @@
Results.EnterNewScope();
AddObjCVisibilityResults(getLangOpts(), Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+
