Re: [PATCH] D20950: [include-fixer] Don't add missing header if the unindentified symbol isn't from the main file.

[Haojian Wu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL271660: [include-fixer] Don't add missing header if the 
unindentified symbol isn't… (authored by hokein).

Changed prior to commit:
  http://reviews.llvm.org/D20950?vs=59496&id=59529#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20950

Files:
  clang-tools-extra/trunk/include-fixer/IncludeFixer.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
@@ -44,6 +44,8 @@
                               llvm::MemoryBuffer::getMemBuffer("\n"));
   InMemoryFileSystem->addFile("dir/otherdir/qux.h", 0,
                               llvm::MemoryBuffer::getMemBuffer("\n"));
+  InMemoryFileSystem->addFile("header.h", 0,
+                              llvm::MemoryBuffer::getMemBuffer("bar b;"));
   return Invocation.run();
 }
 
@@ -186,6 +188,11 @@
             runIncludeFixer("int test = a::b::Green;\n"));
 }
 
+TEST(IncludeFixer, IgnoreSymbolFromHeader) {
+  std::string Code = "#include \"header.h\"";
+  EXPECT_EQ(Code, runIncludeFixer(Code));
+}
+
 // FIXME: add test cases for inserting and sorting multiple headers when
 // include-fixer supports multiple headers insertion.
 TEST(IncludeFixer, InsertAndSortSingleHeader) {
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
@@ -86,6 +86,29 @@
     if (getCompilerInstance().getSema().isSFINAEContext())
       return clang::TypoCorrection();
 
+    // We currently ignore the unidentified symbol which is not from the
+    // main file.
+    //
+    // However, this is not always true due to templates in a non-self 
contained
+    // header, consider the case:
+    //
+    //   // header.h
+    //   template <typename T>
+    //   class Foo {
+    //     T t;
+    //   };
+    //
+    //   // test.cc
+    //   // We need to add <bar.h> in test.cc instead of header.h.
+    //   class Bar;
+    //   Foo<Bar> foo;
+    //
+    // FIXME: Add the missing header to the header file where the symbol comes
+    // from.
+    if (!getCompilerInstance().getSourceManager().isWrittenInMainFile(
+            Typo.getLoc()))
+      return clang::TypoCorrection();
+
     std::string TypoScopeString;
     if (S) {
       // FIXME: Currently we only use namespace contexts. Use other context



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
@@ -44,6 +44,8 @@
                               llvm::MemoryBuffer::getMemBuffer("\n"));
   InMemoryFileSystem->addFile("dir/otherdir/qux.h", 0,
                               llvm::MemoryBuffer::getMemBuffer("\n"));
+  InMemoryFileSystem->addFile("header.h", 0,
+                              llvm::MemoryBuffer::getMemBuffer("bar b;"));
   return Invocation.run();
 }
 
@@ -186,6 +188,11 @@
             runIncludeFixer("int test = a::b::Green;\n"));
 }
 
+TEST(IncludeFixer, IgnoreSymbolFromHeader) {
+  std::string Code = "#include \"header.h\"";
+  EXPECT_EQ(Code, runIncludeFixer(Code));
+}
+
 // FIXME: add test cases for inserting and sorting multiple headers when
 // include-fixer supports multiple headers insertion.
 TEST(IncludeFixer, InsertAndSortSingleHeader) {
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
@@ -86,6 +86,29 @@
     if (getCompilerInstance().getSema().isSFINAEContext())
       return clang::TypoCorrection();
 
+    // We currently ignore the unidentified symbol which is not from the
+    // main file.
+    //
+    // However, this is not always true due to templates in a non-self contained
+    // header, consider the case:
+    //
+    //   // header.h
+    //   template <typename T>
+    //   class Foo {
+    //     T t;
+    //   };
+    //
+    //   // test.cc
+    //   // We need to add <bar.h> in test.cc instead of header.h.
+    //   class Bar;
+    //   Foo<Bar> foo;
+    //
+    // FIXME: Add the missing header to the header file where the symbol comes
+    // from.
+    if (!getCompilerInstance().getSourceManager().isWrittenInMainFile(
+            Typo.getLoc()))
+      return clang::TypoCorrection();
+
     std::string TypoScopeString;
     if (S) {
       // FIXME: Currently we only use namespace contexts. Use other context

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