Re: [PATCH] D20816: [include-fixer] use clang-format cleaner to insert header.

[Haojian Wu via cfe-commits]

hokein added inline comments.


Comment at: include-fixer/IncludeFixer.cpp:29
@@ -28,3 +28,3 @@
 
 class Action;
 

The forward declaration can be removed too.


Repository:
  rL LLVM

http://reviews.llvm.org/D20816



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

Re: [PATCH] D20816: [include-fixer] use clang-format cleaner to insert header.

[Eric Liu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL271287: [include-fixer] use clang-format cleaner to insert 
header. (authored by ioeric).

Changed prior to commit:
  http://reviews.llvm.org/D20816?vs=59067=59069#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20816

Files:
  clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
  clang-tools-extra/trunk/include-fixer/IncludeFixer.h
  clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
  clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.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
@@ -78,21 +78,22 @@
 return Code;
   tooling::Replacements Replacements =
   clang::include_fixer::createInsertHeaderReplacements(
-  Code, "input.cc", FixerContext.Headers.front(),
-  FixerContext.FirstIncludeOffset);
+  Code, "input.cc", FixerContext.Headers.front());
   clang::RewriterTestContext Context;
   clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
   clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
   return Context.getRewrittenText(ID);
 }
 
 TEST(IncludeFixer, Typo) {
-  EXPECT_EQ("#include \n\nstd::string foo;\n",
+  EXPECT_EQ("#include \nstd::string foo;\n",
 runIncludeFixer("std::string foo;\n"));
 
+  // FIXME: the current version of include-fixer does not get this test case
+  // right - header should be inserted before definition.
   EXPECT_EQ(
-  "// comment\n#include \"foo.h\"\n#include \nstd::string foo;\n"
-  "#include \"dir/bar.h\"\n",
+  "// comment\n#include \"foo.h\"\nstd::string foo;\n"
+  "#include \"dir/bar.h\"\n#include \n",
   runIncludeFixer("// comment\n#include \"foo.h\"\nstd::string foo;\n"
   "#include \"dir/bar.h\"\n"));
 
@@ -106,11 +107,11 @@
   // string without "std::" can also be fixed since fixed db results go through
   // SymbolIndexManager, and SymbolIndexManager matches unqualified identifiers
   // too.
-  EXPECT_EQ("#include \n\nstring foo;\n",
+  EXPECT_EQ("#include \nstring foo;\n",
 runIncludeFixer("string foo;\n"));
 
   // Fully qualified name.
-  EXPECT_EQ("#include \n\n::std::string foo;\n",
+  EXPECT_EQ("#include \n::std::string foo;\n",
 runIncludeFixer("::std::string foo;\n"));
   // Should not match std::string.
   EXPECT_EQ("::string foo;\n", runIncludeFixer("::string foo;\n"));
@@ -126,24 +127,24 @@
 
 TEST(IncludeFixer, MinimizeInclude) {
   std::vector IncludePath = {"-Idir/"};
-  EXPECT_EQ("#include \"otherdir/qux.h\"\n\na::b::foo bar;\n",
+  EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n",
 runIncludeFixer("a::b::foo bar;\n", IncludePath));
 
   IncludePath = {"-isystemdir"};
-  EXPECT_EQ("#include \n\na::b::foo bar;\n",
+  EXPECT_EQ("#include \na::b::foo bar;\n",
 runIncludeFixer("a::b::foo bar;\n", IncludePath));
 
   IncludePath = {"-iquotedir"};
-  EXPECT_EQ("#include \"otherdir/qux.h\"\n\na::b::foo bar;\n",
+  EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n",
 runIncludeFixer("a::b::foo bar;\n", IncludePath));
 
   IncludePath = {"-Idir", "-Idir/otherdir"};
-  EXPECT_EQ("#include \"qux.h\"\n\na::b::foo bar;\n",
+  EXPECT_EQ("#include \"qux.h\"\na::b::foo bar;\n",
 runIncludeFixer("a::b::foo bar;\n", IncludePath));
 }
 
 TEST(IncludeFixer, NestedName) {
-  EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n\n"
+  EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n"
 "int x = a::b::foo(0);\n",
 runIncludeFixer("int x = a::b::foo(0);\n"));
 
@@ -153,33 +154,35 @@
   EXPECT_EQ("#define FOO(x) a::##x\nint x = FOO(b::foo);\n",
 runIncludeFixer("#define FOO(x) a::##x\nint x = FOO(b::foo);\n"));
 
-  EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n\n"
-"namespace a {}\nint a = a::b::foo(0);\n",
+  // The empty namespace is cleaned up by clang-format after include-fixer
+  // finishes.
+  EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n"
+"\nint a = a::b::foo(0);\n",
 runIncludeFixer("namespace a {}\nint a = a::b::foo(0);\n"));
 }
 
 TEST(IncludeFixer, MultipleMissingSymbols) {
-  EXPECT_EQ("#include \n\nstd::string bar;\nstd::sting foo;\n",
+  EXPECT_EQ("#include \nstd::string bar;\nstd::sting foo;\n",
 runIncludeFixer("std::string bar;\nstd::sting foo;\n"));
 }
 
 TEST(IncludeFixer, ScopedNamespaceSymbols) {
-  EXPECT_EQ("#include \"bar.h\"\n\nnamespace a {\nb::bar b;\n}",
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nb::bar b;\n}",
 runIncludeFixer("namespace a {\nb::bar b;\n}"));
-  EXPECT_EQ("#include \"bar.h\"\n\nnamespace A {\na::b::bar b;\n}",

Re: [PATCH] D20816: [include-fixer] use clang-format cleaner to insert header.

[Benjamin Kramer via cfe-commits]

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

Look at all this annoying preprocessor code going away! I love it.


http://reviews.llvm.org/D20816



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