Re: [PATCH] D20424: [include-fixer] Make search handle fully qualified names correctly.

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL270055: [include-fixer] Make search handle fully qualified 
names correctly. (authored by d0k).

Changed prior to commit:
  http://reviews.llvm.org/D20424?vs=57766&id=57772#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20424

Files:
  clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
  clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

Index: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
===
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
@@ -103,6 +103,12 @@
   // too.
   EXPECT_EQ("#include \n\nstring foo;\n",
 runIncludeFixer("string foo;\n"));
+
+  // Fully qualified name.
+  EXPECT_EQ("#include \n\n::std::string foo;\n",
+runIncludeFixer("::std::string foo;\n"));
+  // Should not match std::string.
+  EXPECT_EQ("::string foo;\n", runIncludeFixer("::string foo;\n"));
 }
 
 TEST(IncludeFixer, IncompleteType) {
Index: clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
===
--- clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
+++ clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
@@ -24,6 +24,12 @@
   llvm::SmallVector Names;
   Identifier.split(Names, "::");
 
+  bool IsFullyQualified = false;
+  if (Identifier.startswith("::")) {
+Names.erase(Names.begin()); // Drop first (empty) element.
+IsFullyQualified = true;
+  }
+
   // As long as we don't find a result keep stripping name parts from the end.
   // This is to support nested classes which aren't recorded in the database.
   // Eventually we will either hit a class (namespaces aren't in the database
@@ -61,6 +67,11 @@
   }
 }
 
+// If the name was qualified we only want to add results if we 
evaluated
+// all contexts.
+if (IsFullyQualified)
+  IsMatched &= (SymbolContext == Symbol.getContexts().end());
+
 // FIXME: Support full match. At this point, we only find symbols in
 // database which end with the same contexts with the identifier.
 if (IsMatched && IdentiferContext == Names.rend()) {


Index: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
===
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
@@ -103,6 +103,12 @@
   // too.
   EXPECT_EQ("#include \n\nstring foo;\n",
 runIncludeFixer("string foo;\n"));
+
+  // Fully qualified name.
+  EXPECT_EQ("#include \n\n::std::string foo;\n",
+runIncludeFixer("::std::string foo;\n"));
+  // Should not match std::string.
+  EXPECT_EQ("::string foo;\n", runIncludeFixer("::string foo;\n"));
 }
 
 TEST(IncludeFixer, IncompleteType) {
Index: clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
===
--- clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
+++ clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
@@ -24,6 +24,12 @@
   llvm::SmallVector Names;
   Identifier.split(Names, "::");
 
+  bool IsFullyQualified = false;
+  if (Identifier.startswith("::")) {
+Names.erase(Names.begin()); // Drop first (empty) element.
+IsFullyQualified = true;
+  }
+
   // As long as we don't find a result keep stripping name parts from the end.
   // This is to support nested classes which aren't recorded in the database.
   // Eventually we will either hit a class (namespaces aren't in the database
@@ -61,6 +67,11 @@
   }
 }
 
+// If the name was qualified we only want to add results if we evaluated
+// all contexts.
+if (IsFullyQualified)
+  IsMatched &= (SymbolContext == Symbol.getContexts().end());
+
 // FIXME: Support full match. At this point, we only find symbols in
 // database which end with the same contexts with the identifier.
 if (IsMatched && IdentiferContext == Names.rend()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20424: [include-fixer] Make search handle fully qualified names correctly.

[Haojian Wu via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM with one nit.



Comment at: include-fixer/SymbolIndexManager.cpp:73
@@ +72,3 @@
+if (IsFullyQualified)
+  IsMatched &= SymbolContext == Symbol.getContexts().end();
+

Might be `IsMatched &= (SymbolContext == Symbol.getContexts().end());` for 
reading clearly?


http://reviews.llvm.org/D20424



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D20424: [include-fixer] Make search handle fully qualified names correctly.

[Benjamin Kramer via cfe-commits]

bkramer created this revision.
bkramer added reviewers: hokein, ioeric.
bkramer added a subscriber: cfe-commits.

If a search string starts with "::" we don't want to return any results
for suffixes of that string.

http://reviews.llvm.org/D20424

Files:
  include-fixer/SymbolIndexManager.cpp
  unittests/include-fixer/IncludeFixerTest.cpp

Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -103,6 +103,12 @@
   // too.
   EXPECT_EQ("#include \n\nstring foo;\n",
 runIncludeFixer("string foo;\n"));
+
+  // Fully qualified name.
+  EXPECT_EQ("#include \n\n::std::string foo;\n",
+runIncludeFixer("::std::string foo;\n"));
+  // Should not match std::string.
+  EXPECT_EQ("::string foo;\n", runIncludeFixer("::string foo;\n"));
 }
 
 TEST(IncludeFixer, IncompleteType) {
Index: include-fixer/SymbolIndexManager.cpp
===
--- include-fixer/SymbolIndexManager.cpp
+++ include-fixer/SymbolIndexManager.cpp
@@ -24,6 +24,12 @@
   llvm::SmallVector Names;
   Identifier.split(Names, "::");
 
+  bool IsFullyQualified = false;
+  if (Identifier.startswith("::")) {
+Names.erase(Names.begin()); // Drop first (empty) element.
+IsFullyQualified = true;
+  }
+
   // As long as we don't find a result keep stripping name parts from the end.
   // This is to support nested classes which aren't recorded in the database.
   // Eventually we will either hit a class (namespaces aren't in the database
@@ -61,6 +67,11 @@
   }
 }
 
+// If the name was qualified we only want to add results if we 
evaluated
+// all contexts.
+if (IsFullyQualified)
+  IsMatched &= SymbolContext == Symbol.getContexts().end();
+
 // FIXME: Support full match. At this point, we only find symbols in
 // database which end with the same contexts with the identifier.
 if (IsMatched && IdentiferContext == Names.rend()) {


Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -103,6 +103,12 @@
   // too.
   EXPECT_EQ("#include \n\nstring foo;\n",
 runIncludeFixer("string foo;\n"));
+
+  // Fully qualified name.
+  EXPECT_EQ("#include \n\n::std::string foo;\n",
+runIncludeFixer("::std::string foo;\n"));
+  // Should not match std::string.
+  EXPECT_EQ("::string foo;\n", runIncludeFixer("::string foo;\n"));
 }
 
 TEST(IncludeFixer, IncompleteType) {
Index: include-fixer/SymbolIndexManager.cpp
===
--- include-fixer/SymbolIndexManager.cpp
+++ include-fixer/SymbolIndexManager.cpp
@@ -24,6 +24,12 @@
   llvm::SmallVector Names;
   Identifier.split(Names, "::");
 
+  bool IsFullyQualified = false;
+  if (Identifier.startswith("::")) {
+Names.erase(Names.begin()); // Drop first (empty) element.
+IsFullyQualified = true;
+  }
+
   // As long as we don't find a result keep stripping name parts from the end.
   // This is to support nested classes which aren't recorded in the database.
   // Eventually we will either hit a class (namespaces aren't in the database
@@ -61,6 +67,11 @@
   }
 }
 
+// If the name was qualified we only want to add results if we evaluated
+// all contexts.
+if (IsFullyQualified)
+  IsMatched &= SymbolContext == Symbol.getContexts().end();
+
 // FIXME: Support full match. At this point, we only find symbols in
 // database which end with the same contexts with the identifier.
 if (IsMatched && IdentiferContext == Names.rend()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits