Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL275279: [include-fixer] Implement adding missing namespace
qualifiers in vim… (authored by hokein).
Changed prior to commit:
http://reviews.llvm.org/D22299?vs=63808=63818#toc
Repository:
rL LLVM
http://reviews.llvm.org/D22299
Files:
clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
clang-tools-extra/trunk/test/include-fixer/ranking.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
@@ -84,18 +84,19 @@
std::string FakeFileName = "input.cc";
runOnCode(, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getMatchedSymbols().empty())
+ if (FixerContext.getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createInsertHeaderReplacements(
- Code, FakeFileName, FixerContext.getHeaders().front());
+ Code, FakeFileName, FixerContext.getHeaderInfos().front().Header);
EXPECT_TRUE(static_cast(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
return "";
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
- if (FixerContext.getSymbolRange().getLength() > 0)
-Replaces->insert(FixerContext.createSymbolReplacement(FakeFileName, 0));
+ Replaces->insert({FakeFileName, FixerContext.getSymbolRange().getOffset(),
+FixerContext.getSymbolRange().getLength(),
+FixerContext.getHeaderInfos().front().QualifiedName});
clang::tooling::applyAllReplacements(*Replaces, Context.Rewrite);
return Context.getRewrittenText(ID);
}
Index: clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
@@ -13,40 +13,27 @@
namespace clang {
namespace include_fixer {
-IncludeFixerContext::IncludeFixerContext(
-llvm::StringRef Name, llvm::StringRef ScopeQualifiers,
-const std::vector Symbols,
-tooling::Range Range)
-: SymbolIdentifier(Name), SymbolScopedQualifiers(ScopeQualifiers),
- MatchedSymbols(Symbols), SymbolRange(Range) {
- // Deduplicate headers, so that we don't want to suggest the same header
- // twice.
- for (const auto : MatchedSymbols)
-Headers.push_back(Symbol.getFilePath());
- Headers.erase(std::unique(Headers.begin(), Headers.end(),
-[](const std::string , const std::string ) {
- return A == B;
-}),
-Headers.end());
-}
+namespace {
-tooling::Replacement
-IncludeFixerContext::createSymbolReplacement(llvm::StringRef FilePath,
- size_t Idx) {
- assert(Idx < MatchedSymbols.size());
+std::string createQualifiedNameForReplacement(
+llvm::StringRef RawSymbolName,
+llvm::StringRef SymbolScopedQualifiers,
+const find_all_symbols::SymbolInfo ) {
// No need to add missing qualifiers if SymbolIndentifer has a global scope
// operator "::".
- if (getSymbolIdentifier().startswith("::"))
-return tooling::Replacement();
- std::string QualifiedName = MatchedSymbols[Idx].getQualifiedName();
+ if (RawSymbolName.startswith("::"))
+return RawSymbolName;
+
+ std::string QualifiedName = MatchedSymbol.getQualifiedName();
+
// For nested classes, the qualified name constructed from database misses
// some stripped qualifiers, because when we search a symbol in database,
// we strip qualifiers from the end until we find a result. So append the
// missing stripped qualifiers here.
//
// Get stripped qualifiers.
llvm::SmallVector SymbolQualifiers;
- getSymbolIdentifier().split(SymbolQualifiers, "::");
+ RawSymbolName.split(SymbolQualifiers, "::");
std::string StrippedQualifiers;
while (!SymbolQualifiers.empty() &&
!llvm::StringRef(QualifiedName).endswith(SymbolQualifiers.back())) {
@@ -56,9 +43,30 @@
// Append the missing stripped qualifiers.
std::string FullyQualifiedName = QualifiedName + StrippedQualifiers;
auto pos = FullyQualifiedName.find(SymbolScopedQualifiers);
- return {FilePath, SymbolRange.getOffset(), SymbolRange.getLength(),
- FullyQualifiedName.substr(
- pos ==
Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
hokein updated this revision to Diff 63808.
hokein marked an inline comment as done.
hokein added a comment.
Fix nits.
http://reviews.llvm.org/D22299
Files:
include-fixer/IncludeFixerContext.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
test/include-fixer/ranking.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -84,18 +84,19 @@
std::string FakeFileName = "input.cc";
runOnCode(, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getMatchedSymbols().empty())
+ if (FixerContext.getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createInsertHeaderReplacements(
- Code, FakeFileName, FixerContext.getHeaders().front());
+ Code, FakeFileName, FixerContext.getHeaderInfos().front().Header);
EXPECT_TRUE(static_cast(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
return "";
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
- if (FixerContext.getSymbolRange().getLength() > 0)
-Replaces->insert(FixerContext.createSymbolReplacement(FakeFileName, 0));
+ Replaces->insert({FakeFileName, FixerContext.getSymbolRange().getOffset(),
+FixerContext.getSymbolRange().getLength(),
+FixerContext.getHeaderInfos().front().QualifiedName});
clang::tooling::applyAllReplacements(*Replaces, Context.Rewrite);
return Context.getRewrittenText(ID);
}
Index: test/include-fixer/ranking.cpp
===
--- test/include-fixer/ranking.cpp
+++ test/include-fixer/ranking.cpp
@@ -1,5 +1,10 @@
// RUN: clang-include-fixer -db=yaml -input=%S/Inputs/fake_yaml_db.yaml -output-headers %s -- | FileCheck %s
-// CHECK: "Headers": [ "\"../include/bar.h\"", "\"../include/zbar.h\"" ]
+// CHECK: "HeaderInfos": [
+// CHECK-NEXT: {"Header": "\"../include/bar.h\"",
+// CHECK-NEXT: "QualifiedName": "b::a::bar"},
+// CHECK-NEXT: {"Header": "\"../include/zbar.h\"",
+// CHECK-NEXT: "QualifiedName": "b::a::bar"}
+// CHECK-NEXT:]
bar b;
Index: test/include-fixer/commandline_options.cpp
===
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,11 +1,16 @@
// REQUIRES: shell
-// RUN: sed -e 's#//.*$##' %s > %t.cpp
-// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Headers: ["\"foo.h\""]}' %t.cpp | FileCheck %s -check-prefix=CHECK
+// RUN: echo "foo f;" > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
-// CHECK-HEADERS: "Headers": [ "\"foo.h\"", "\"bar.h\"" ]
+// CHECK: "HeaderInfos": [
+// CHECK-NEXT: {"Header": "\"foo.h\"",
+// CHECK-NEXT: "QualifiedName": "foo"},
+// CHECK-NEXT: {"Header": "\"bar.h\"",
+// CHECK-NEXT: "QualifiedName": "foo"}
+// CHECK-NEXT:]
//
-// CHECK: #include "foo.h"
-// CHECK: foo f;
-
-foo f;
+// CHECK-CODE: #include "foo.h"
+// CHECK-CODE: foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -115,30 +115,44 @@
include_fixer_context = json.loads(stdout)
symbol = include_fixer_context["SymbolIdentifier"]
- headers = include_fixer_context["Headers"]
+ # The header_infos is already sorted by include-fixer.
+ header_infos = include_fixer_context["HeaderInfos"]
+ # Deduplicate headers while keeping the order, so that the same header would
+ # not be suggested twice.
+ unique_headers = []
+ seen = set()
+ for header_info in header_infos:
+header = header_info["Header"]
+if header not in
Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. One nit, LGTM. Comment at: include-fixer/tool/ClangIncludeFixer.cpp:277 @@ +276,3 @@ +// +// Check whether all elements in HeaderInfos have the qualified name. +bool IsUniqueQualifiedName = std::equal( the **same** qualified name http://reviews.llvm.org/D22299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
hokein added inline comments. Comment at: include-fixer/tool/ClangIncludeFixer.cpp:252 @@ +251,3 @@ +assert(!HeaderInfos.empty()); +// Only accept an unique header. +bool IsUniqueHeader = std::equal( bkramer wrote: > In that case adjacent_find was the right choice, but std::equal also works, I > read this wrong. There are multiple ways of doing this using standard > algorithms and none of them is really obvious. Please add a comment here that > you're checking that all elements are equal. Done. To me, `equal` is more obvious than `adjacent_find`. http://reviews.llvm.org/D22299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
hokein updated this revision to Diff 63799.
hokein marked 2 inline comments as done.
hokein added a comment.
Add comments for std::equal.
http://reviews.llvm.org/D22299
Files:
include-fixer/IncludeFixerContext.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
test/include-fixer/ranking.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -84,18 +84,19 @@
std::string FakeFileName = "input.cc";
runOnCode(, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getMatchedSymbols().empty())
+ if (FixerContext.getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createInsertHeaderReplacements(
- Code, FakeFileName, FixerContext.getHeaders().front());
+ Code, FakeFileName, FixerContext.getHeaderInfos().front().Header);
EXPECT_TRUE(static_cast(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
return "";
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
- if (FixerContext.getSymbolRange().getLength() > 0)
-Replaces->insert(FixerContext.createSymbolReplacement(FakeFileName, 0));
+ Replaces->insert({FakeFileName, FixerContext.getSymbolRange().getOffset(),
+FixerContext.getSymbolRange().getLength(),
+FixerContext.getHeaderInfos().front().QualifiedName});
clang::tooling::applyAllReplacements(*Replaces, Context.Rewrite);
return Context.getRewrittenText(ID);
}
Index: test/include-fixer/ranking.cpp
===
--- test/include-fixer/ranking.cpp
+++ test/include-fixer/ranking.cpp
@@ -1,5 +1,10 @@
// RUN: clang-include-fixer -db=yaml -input=%S/Inputs/fake_yaml_db.yaml -output-headers %s -- | FileCheck %s
-// CHECK: "Headers": [ "\"../include/bar.h\"", "\"../include/zbar.h\"" ]
+// CHECK: "HeaderInfos": [
+// CHECK-NEXT: {"Header": "\"../include/bar.h\"",
+// CHECK-NEXT: "QualifiedName": "b::a::bar"},
+// CHECK-NEXT: {"Header": "\"../include/zbar.h\"",
+// CHECK-NEXT: "QualifiedName": "b::a::bar"}
+// CHECK-NEXT:]
bar b;
Index: test/include-fixer/commandline_options.cpp
===
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,11 +1,16 @@
// REQUIRES: shell
-// RUN: sed -e 's#//.*$##' %s > %t.cpp
-// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Headers: ["\"foo.h\""]}' %t.cpp | FileCheck %s -check-prefix=CHECK
+// RUN: echo "foo f;" > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
-// CHECK-HEADERS: "Headers": [ "\"foo.h\"", "\"bar.h\"" ]
+// CHECK: "HeaderInfos": [
+// CHECK-NEXT: {"Header": "\"foo.h\"",
+// CHECK-NEXT: "QualifiedName": "foo"},
+// CHECK-NEXT: {"Header": "\"bar.h\"",
+// CHECK-NEXT: "QualifiedName": "foo"}
+// CHECK-NEXT:]
//
-// CHECK: #include "foo.h"
-// CHECK: foo f;
-
-foo f;
+// CHECK-CODE: #include "foo.h"
+// CHECK-CODE: foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -115,30 +115,44 @@
include_fixer_context = json.loads(stdout)
symbol = include_fixer_context["SymbolIdentifier"]
- headers = include_fixer_context["Headers"]
+ # The header_infos is already sorted by include-fixer.
+ header_infos = include_fixer_context["HeaderInfos"]
+ # Deduplicate headers while keeping the order, so that the same header would
+ # not be suggested twice.
+ unique_headers = []
+ seen = set()
+ for header_info in header_infos:
+header = header_info["Header"]
+
Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
bkramer added inline comments. Comment at: include-fixer/tool/ClangIncludeFixer.cpp:252 @@ +251,3 @@ +assert(!HeaderInfos.empty()); +// Only accept an unique header. +bool IsUniqueHeader = std::equal( In that case adjacent_find was the right choice, but std::equal also works, I read this wrong. There are multiple ways of doing this using standard algorithms and none of them is really obvious. Please add a comment here that you're checking that all elements are equal. http://reviews.llvm.org/D22299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
hokein marked an inline comment as done. Comment at: include-fixer/tool/ClangIncludeFixer.cpp:252 @@ +251,3 @@ +// Only accept an unique header. +bool IsUniqueHeader = +std::adjacent_find(HeaderInfos.begin(), HeaderInfos.end(), You are right. `adjancent_find` is not correct algorithm here. What I want to do is to check all elements' headers in `HeaderInfos` are equal. Change to `std::equal`, the same below. Comment at: include-fixer/tool/clang-include-fixer.py:119 @@ +118,3 @@ + header_infos = include_fixer_context["HeaderInfos"] + # Reduplicate headers, so that the same header would not be suggested twice. + headers = list(set([header_info["Header"] for header_info in header_infos])) Should be `Deduplicate`. http://reviews.llvm.org/D22299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
hokein updated this revision to Diff 63796.
hokein marked 6 inline comments as done.
hokein added a comment.
Address review comments.
http://reviews.llvm.org/D22299
Files:
include-fixer/IncludeFixerContext.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
test/include-fixer/ranking.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -84,18 +84,19 @@
std::string FakeFileName = "input.cc";
runOnCode(, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getMatchedSymbols().empty())
+ if (FixerContext.getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createInsertHeaderReplacements(
- Code, FakeFileName, FixerContext.getHeaders().front());
+ Code, FakeFileName, FixerContext.getHeaderInfos().front().Header);
EXPECT_TRUE(static_cast(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
return "";
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
- if (FixerContext.getSymbolRange().getLength() > 0)
-Replaces->insert(FixerContext.createSymbolReplacement(FakeFileName, 0));
+ Replaces->insert({FakeFileName, FixerContext.getSymbolRange().getOffset(),
+FixerContext.getSymbolRange().getLength(),
+FixerContext.getHeaderInfos().front().QualifiedName});
clang::tooling::applyAllReplacements(*Replaces, Context.Rewrite);
return Context.getRewrittenText(ID);
}
Index: test/include-fixer/ranking.cpp
===
--- test/include-fixer/ranking.cpp
+++ test/include-fixer/ranking.cpp
@@ -1,5 +1,10 @@
// RUN: clang-include-fixer -db=yaml -input=%S/Inputs/fake_yaml_db.yaml -output-headers %s -- | FileCheck %s
-// CHECK: "Headers": [ "\"../include/bar.h\"", "\"../include/zbar.h\"" ]
+// CHECK: "HeaderInfos": [
+// CHECK-NEXT: {"Header": "\"../include/bar.h\"",
+// CHECK-NEXT: "QualifiedName": "b::a::bar"},
+// CHECK-NEXT: {"Header": "\"../include/zbar.h\"",
+// CHECK-NEXT: "QualifiedName": "b::a::bar"}
+// CHECK-NEXT:]
bar b;
Index: test/include-fixer/commandline_options.cpp
===
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,11 +1,16 @@
// REQUIRES: shell
-// RUN: sed -e 's#//.*$##' %s > %t.cpp
-// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Headers: ["\"foo.h\""]}' %t.cpp | FileCheck %s -check-prefix=CHECK
+// RUN: echo "foo f;" > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Range: {Offset: 0, Length: 3}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
-// CHECK-HEADERS: "Headers": [ "\"foo.h\"", "\"bar.h\"" ]
+// CHECK: "HeaderInfos": [
+// CHECK-NEXT: {"Header": "\"foo.h\"",
+// CHECK-NEXT: "QualifiedName": "foo"},
+// CHECK-NEXT: {"Header": "\"bar.h\"",
+// CHECK-NEXT: "QualifiedName": "foo"}
+// CHECK-NEXT:]
//
-// CHECK: #include "foo.h"
-// CHECK: foo f;
-
-foo f;
+// CHECK-CODE: #include "foo.h"
+// CHECK-CODE: foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -115,30 +115,44 @@
include_fixer_context = json.loads(stdout)
symbol = include_fixer_context["SymbolIdentifier"]
- headers = include_fixer_context["Headers"]
+ # The header_infos is already sorted by include-fixer.
+ header_infos = include_fixer_context["HeaderInfos"]
+ # Deduplicate headers while keeping the order, so that the same header would
+ # not be suggested twice.
+ unique_headers = []
+ seen = set()
+ for header_info in header_infos:
+header = header_info["Header"]
+
Re: [PATCH] D22299: [include-fixer] Implement adding missing namespace qualifiers in vim integration.
bkramer added inline comments.
Comment at: include-fixer/IncludeFixerContext.cpp:54
@@ +53,3 @@
+llvm::StringRef Name, llvm::StringRef ScopeQualifiers,
+const std::vector Symbols,
+tooling::Range Range)
Drop the const ...
Comment at: include-fixer/IncludeFixerContext.cpp:57
@@ +56,3 @@
+: SymbolIdentifier(Name), SymbolScopedQualifiers(ScopeQualifiers),
+ MatchedSymbols(Symbols), SymbolRange(Range) {
+ for (const auto : MatchedSymbols) {
... use std::move to move Symbols into place.
Comment at: include-fixer/IncludeFixerContext.h:32
@@ +31,3 @@
+std::string Header;
+/// \brief A symbol name with completed namespace qulifiers which will
+/// replace the original symbol.
Typo: qulifiers
Comment at: include-fixer/IncludeFixerContext.h:65
@@ +64,3 @@
+
+ /// \brief The header informations.
+ std::vector HeaderInfos;
s/informations/information/
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:252
@@ +251,3 @@
+// Only accept an unique header.
+bool IsUniqueHeader =
+std::adjacent_find(HeaderInfos.begin(), HeaderInfos.end(),
Is it guaranteed that two headers with the same name are adjacent in the
HeaderInfos vector? If so, point that out in a comment, otherwise adjacent_find
isn't the right algorithm here.
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:259
@@ -209,1 +258,3 @@
+if (!IsUniqueHeader) {
+ errs() << "Expect exactly an unique header.\n";
return 1;
"Expected exactly one unique header"?
Comment at: include-fixer/tool/clang-include-fixer.py:119
@@ +118,3 @@
+ header_infos = include_fixer_context["HeaderInfos"]
+ # Reduplicate headers, so that the same header would not be suggested twice.
+ headers = list(set([header_info["Header"] for header_info in header_infos]))
Reduplicate?
http://reviews.llvm.org/D22299
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
