Fix CodeCompletion & TypoCorrection when combining a PCH with Modules

This commit fixes the IdentifierIterator to actually include identifiers
from a PCH or precompiled preamble when there is also a global module
index. This was causing code-completion (outside of C++) and
typo-correction to be missing global identifiers defined in the
PCH/preamble. Typo-correction has been broken since we first started
using the module index, whereas code-completion only started relying on
identifier iterator in r232793.

rdar://problem/25642879

llvm-svn: 268471

diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index aa672b3..e33cccb 100644 (file)
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7014,19 +7014,20 @@ namespace clang {
     /// the current AST file.
     ASTIdentifierLookupTable::key_iterator End;
 
+    /// \brief Whether to skip any modules in the ASTReader.
+    bool SkipModules;
+
   public:
-    explicit ASTIdentifierIterator(const ASTReader &Reader);
+    explicit ASTIdentifierIterator(const ASTReader &Reader,
+                                   bool SkipModules = false);
 
     StringRef Next() override;
   };
 }
 
-ASTIdentifierIterator::ASTIdentifierIterator(const ASTReader &Reader)
-  : Reader(Reader), Index(Reader.ModuleMgr.size() - 1) {
-  ASTIdentifierLookupTable *IdTable
-    = (ASTIdentifierLookupTable *)Reader.ModuleMgr[Index].IdentifierLookupTable;
-  Current = IdTable->key_begin();
-  End = IdTable->key_end();
+ASTIdentifierIterator::ASTIdentifierIterator(const ASTReader &Reader,
+                                             bool SkipModules)
+    : Reader(Reader), Index(Reader.ModuleMgr.size()), SkipModules(SkipModules) {
 }
 
 StringRef ASTIdentifierIterator::Next() {
@@ -7036,9 +7037,12 @@ StringRef ASTIdentifierIterator::Next() {
       return StringRef();
 
     --Index;
-    ASTIdentifierLookupTable *IdTable
-      = (ASTIdentifierLookupTable *)Reader.ModuleMgr[Index].
-        IdentifierLookupTable;
+    ModuleFile &F = Reader.ModuleMgr[Index];
+    if (SkipModules && F.isModule())
+      continue;
+
+    ASTIdentifierLookupTable *IdTable =
+        (ASTIdentifierLookupTable *)F.IdentifierLookupTable;
     Current = IdTable->key_begin();
     End = IdTable->key_end();
   }
@@ -7050,9 +7054,42 @@ StringRef ASTIdentifierIterator::Next() {
   return Result;
 }
 
+namespace {
+/// A utility for appending two IdentifierIterators.
+class ChainedIdentifierIterator : public IdentifierIterator {
+  std::unique_ptr<IdentifierIterator> Current;
+  std::unique_ptr<IdentifierIterator> Queued;
+
+public:
+  ChainedIdentifierIterator(std::unique_ptr<IdentifierIterator> First,
+                            std::unique_ptr<IdentifierIterator> Second)
+      : Current(std::move(First)), Queued(std::move(Second)) {}
+
+  StringRef Next() override {
+    if (!Current)
+      return StringRef();
+
+    StringRef result = Current->Next();
+    if (!result.empty())
+      return result;
+
+    // Try the queued iterator, which may itself be empty.
+    Current.reset();
+    std::swap(Current, Queued);
+    return Next();
+  }
+};
+} // end anonymous namespace.
+
 IdentifierIterator *ASTReader::getIdentifiers() {
-  if (!loadGlobalIndex())
-    return GlobalIndex->createIdentifierIterator();
+  if (!loadGlobalIndex()) {
+    std::unique_ptr<IdentifierIterator> ReaderIter(
+        new ASTIdentifierIterator(*this, /*SkipModules=*/true));
+    std::unique_ptr<IdentifierIterator> ModulesIter(
+        GlobalIndex->createIdentifierIterator());
+    return new ChainedIdentifierIterator(std::move(ReaderIter),
+                                         std::move(ModulesIter));
+  }
 
   return new ASTIdentifierIterator(*this);
 }

diff --git a/clang/test/CodeCompletion/Inputs/ModuleA/module.modulemap b/clang/test/CodeCompletion/Inputs/ModuleA/module.modulemap
new file mode 100644 (file)
index 0000000..b0fe1fa
--- /dev/null
+++ b/clang/test/CodeCompletion/Inputs/ModuleA/module.modulemap
@@ -0,0 +1,4 @@
+module ModuleA {
+  header "moduleA.h"
+  export *
+}

diff --git a/clang/test/CodeCompletion/Inputs/ModuleA/moduleA.h b/clang/test/CodeCompletion/Inputs/ModuleA/moduleA.h
new file mode 100644 (file)
index 0000000..f90f56d
--- /dev/null
+++ b/clang/test/CodeCompletion/Inputs/ModuleA/moduleA.h
@@ -0,0 +1 @@
+static int const FROM_MODULE_A = 0;

diff --git a/clang/test/CodeCompletion/Inputs/import_moduleA.h b/clang/test/CodeCompletion/Inputs/import_moduleA.h
new file mode 100644 (file)
index 0000000..e2663f4
--- /dev/null
+++ b/clang/test/CodeCompletion/Inputs/import_moduleA.h
@@ -0,0 +1,2 @@
+#include "ModuleA/moduleA.h"
+static int const FROM_HEADER = 1;

diff --git a/clang/test/CodeCompletion/pch-and-module.m b/clang/test/CodeCompletion/pch-and-module.m
new file mode 100644 (file)
index 0000000..8361448
--- /dev/null
+++ b/clang/test/CodeCompletion/pch-and-module.m
@@ -0,0 +1,37 @@
+#import "import_moduleA.h"
+static const int FROM_IMPL = 2;
+
+void test0(void) {
+  int x = 
+}
+// The lines above this point are sensitive to line/column changes.
+
+// ===--- None
+// RUN: c-index-test -code-completion-at=%s:5:11 %s -I %S/Inputs | FileCheck %s
+
+// ===--- Modules
+// RUN: rm -rf %t && mkdir %t
+// RUN: c-index-test -code-completion-at=%s:5:11 %s -I %S/Inputs -fmodules -fmodules-cache-path=%t/mcp | FileCheck %s
+
+// ===--- PCH
+// RUN: rm -rf %t && mkdir %t
+// RUN: c-index-test -write-pch %t/import_moduleA.pch -x objective-c-header %S/Inputs/import_moduleA.h -I %S/Inputs
+// RUN: c-index-test -code-completion-at=%s:5:11 %s -include-pch %t/import_moduleA.pch -I %S/Inputs | FileCheck %s
+
+// ===--- PCH + Modules
+// RUN: rm -rf %t && mkdir %t
+// RUN: c-index-test -write-pch %t/import_moduleA.pch -x objective-c-header %S/Inputs/import_moduleA.h -fmodules -fmodules-cache-path=%t/mcp -I %S/Inputs
+// RUN: c-index-test -code-completion-at=%s:5:11 %s -include-pch %t/import_moduleA.pch -I %S/Inputs -fmodules -fmodules-cache-path=%t/mcp | FileCheck %s
+
+// ===--- Preamble
+// RUN: rm -rf %t && mkdir %t
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -code-completion-at=%s:5:11 %s -I %S/Inputs | FileCheck %s
+
+// ===--- Preamble + Modules
+// RUN: rm -rf %t
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -code-completion-at=%s:5:11 %s -I %S/Inputs -fmodules -fmodules-cache-path=%t/mcp | FileCheck %s
+
+
+// CHECK: FROM_HEADER
+// CHECK: FROM_IMPL
+// CHECK: FROM_MODULE_A

diff --git a/clang/test/Modules/Inputs/module.map b/clang/test/Modules/Inputs/module.map
index 1249387..66b52e9 100644 (file)
--- a/clang/test/Modules/Inputs/module.map
+++ b/clang/test/Modules/Inputs/module.map
@@ -410,3 +410,5 @@ module MethodPoolString1 {
 module MethodPoolString2 {
   header "MethodPoolString2.h"
 }
+
+module Empty {}

diff --git a/clang/test/Modules/Inputs/typo.h b/clang/test/Modules/Inputs/typo.h
new file mode 100644 (file)
index 0000000..764c00b
--- /dev/null
+++ b/clang/test/Modules/Inputs/typo.h
@@ -0,0 +1,6 @@
+@import Empty;
+
+@interface NSString
++ (id)alloc;
+@end
+

diff --git a/clang/test/Modules/typo.m b/clang/test/Modules/typo.m
new file mode 100644 (file)
index 0000000..7e5108d
--- /dev/null
+++ b/clang/test/Modules/typo.m
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs -x objective-c-header %S/Inputs/typo.h -emit-pch -o %t.pch
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs -include-pch %t.pch %s -verify
+
+void test() {
+  [Nsstring alloc]; // expected-error {{unknown receiver 'Nsstring'; did you mean 'NSString'}}
+                    // expected-note@typo.h:* {{here}}
+}