bkramer created this revision.
bkramer added reviewers: hokein, ioeric.
bkramer added a subscriber: cfe-commits.
If prefix search finds something where nothing can be nested under (e.g.
a variable or macro) don't add it to the result.
This is for cases like:
header.h:
extern int a;
file.cc:
namespace a {
SOME_MACRO
}
We will look up a::SOME_MACRO, which doesn't have any results. Then we
look up 'a' and find something before we ever look up just 'SOME_MACRO'.
With some basic filtering we can avoid this case.
http://reviews.llvm.org/D20960
Files:
include-fixer/SymbolIndexManager.cpp
test/include-fixer/Inputs/fake_yaml_db.yaml
test/include-fixer/prefix_variable.cpp
Index: test/include-fixer/prefix_variable.cpp
===================================================================
--- /dev/null
+++ test/include-fixer/prefix_variable.cpp
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=yaml -input=%p/Inputs/fake_yaml_db.yaml %t.cpp
--
+// RUN: FileCheck %s -input-file=%t.cpp
+
+// CHECK-NOT: #include
+// CHECK: doesnotexist f;
+
+namespace b {
+doesnotexist f;
+}
Index: test/include-fixer/Inputs/fake_yaml_db.yaml
===================================================================
--- test/include-fixer/Inputs/fake_yaml_db.yaml
+++ test/include-fixer/Inputs/fake_yaml_db.yaml
@@ -43,3 +43,11 @@
LineNumber: 1
Type: Class
NumOccurrences: 3
+---
+Name: b
+Contexts:
+FilePath: var.h
+LineNumber: 1
+Type: Variable
+NumOccurrences: 1
+...
Index: include-fixer/SymbolIndexManager.cpp
===================================================================
--- include-fixer/SymbolIndexManager.cpp
+++ include-fixer/SymbolIndexManager.cpp
@@ -66,6 +66,7 @@
// 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
// either) and can report that result.
+ bool TookPrefix = false;
std::vector<std::string> Results;
while (Results.empty() && !Names.empty()) {
std::vector<clang::find_all_symbols::SymbolInfo> Symbols;
@@ -109,6 +110,16 @@
// 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()) {
+ // If we're in a situation where we took a prefix but the thing we
+ // found couldn't possibly have a nested member ignore it.
+ if (TookPrefix &&
+ (Symbol.getSymbolKind() == SymbolInfo::SymbolKind::Function ||
+ Symbol.getSymbolKind() == SymbolInfo::SymbolKind::Variable ||
+ Symbol.getSymbolKind() ==
+ SymbolInfo::SymbolKind::EnumConstantDecl ||
+ Symbol.getSymbolKind() == SymbolInfo::SymbolKind::Macro))
+ continue;
+
// FIXME: file path should never be in the form of <...> or "...",
but
// the unit test with fixed database use <...> file path, which might
// need to be changed.
@@ -122,6 +133,7 @@
}
}
Names.pop_back();
+ TookPrefix = true;
}
return Results;
Index: test/include-fixer/prefix_variable.cpp
===================================================================
--- /dev/null
+++ test/include-fixer/prefix_variable.cpp
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=yaml -input=%p/Inputs/fake_yaml_db.yaml %t.cpp --
+// RUN: FileCheck %s -input-file=%t.cpp
+
+// CHECK-NOT: #include
+// CHECK: doesnotexist f;
+
+namespace b {
+doesnotexist f;
+}
Index: test/include-fixer/Inputs/fake_yaml_db.yaml
===================================================================
--- test/include-fixer/Inputs/fake_yaml_db.yaml
+++ test/include-fixer/Inputs/fake_yaml_db.yaml
@@ -43,3 +43,11 @@
LineNumber: 1
Type: Class
NumOccurrences: 3
+---
+Name: b
+Contexts:
+FilePath: var.h
+LineNumber: 1
+Type: Variable
+NumOccurrences: 1
+...
Index: include-fixer/SymbolIndexManager.cpp
===================================================================
--- include-fixer/SymbolIndexManager.cpp
+++ include-fixer/SymbolIndexManager.cpp
@@ -66,6 +66,7 @@
// 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
// either) and can report that result.
+ bool TookPrefix = false;
std::vector<std::string> Results;
while (Results.empty() && !Names.empty()) {
std::vector<clang::find_all_symbols::SymbolInfo> Symbols;
@@ -109,6 +110,16 @@
// 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()) {
+ // If we're in a situation where we took a prefix but the thing we
+ // found couldn't possibly have a nested member ignore it.
+ if (TookPrefix &&
+ (Symbol.getSymbolKind() == SymbolInfo::SymbolKind::Function ||
+ Symbol.getSymbolKind() == SymbolInfo::SymbolKind::Variable ||
+ Symbol.getSymbolKind() ==
+ SymbolInfo::SymbolKind::EnumConstantDecl ||
+ Symbol.getSymbolKind() == SymbolInfo::SymbolKind::Macro))
+ continue;
+
// FIXME: file path should never be in the form of <...> or "...", but
// the unit test with fixed database use <...> file path, which might
// need to be changed.
@@ -122,6 +133,7 @@
}
}
Names.pop_back();
+ TookPrefix = true;
}
return Results;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
