r268471 - Fix CodeCompletion & TypoCorrection when combining a PCH with Modules

Ben Langmuir via cfe-commits cfe-commits at lists.llvm.org
Tue May 3 17:53:14 PDT 2016


Author: benlangmuir
Date: Tue May  3 19:53:13 2016
New Revision: 268471

URL: http://llvm.org/viewvc/llvm-project?rev=268471&view=rev
Log:
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

Added:
    cfe/trunk/test/CodeCompletion/Inputs/ModuleA/
    cfe/trunk/test/CodeCompletion/Inputs/ModuleA/module.modulemap
    cfe/trunk/test/CodeCompletion/Inputs/ModuleA/moduleA.h
    cfe/trunk/test/CodeCompletion/Inputs/import_moduleA.h
    cfe/trunk/test/CodeCompletion/pch-and-module.m
    cfe/trunk/test/Modules/Inputs/typo.h
    cfe/trunk/test/Modules/typo.m
Modified:
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/test/Modules/Inputs/module.map

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=268471&r1=268470&r2=268471&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue May  3 19:53:13 2016
@@ -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);
 }

Added: cfe/trunk/test/CodeCompletion/Inputs/ModuleA/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/Inputs/ModuleA/module.modulemap?rev=268471&view=auto
==============================================================================
--- cfe/trunk/test/CodeCompletion/Inputs/ModuleA/module.modulemap (added)
+++ cfe/trunk/test/CodeCompletion/Inputs/ModuleA/module.modulemap Tue May  3 19:53:13 2016
@@ -0,0 +1,4 @@
+module ModuleA {
+  header "moduleA.h"
+  export *
+}

Added: cfe/trunk/test/CodeCompletion/Inputs/ModuleA/moduleA.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/Inputs/ModuleA/moduleA.h?rev=268471&view=auto
==============================================================================
--- cfe/trunk/test/CodeCompletion/Inputs/ModuleA/moduleA.h (added)
+++ cfe/trunk/test/CodeCompletion/Inputs/ModuleA/moduleA.h Tue May  3 19:53:13 2016
@@ -0,0 +1 @@
+static int const FROM_MODULE_A = 0;

Added: cfe/trunk/test/CodeCompletion/Inputs/import_moduleA.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/Inputs/import_moduleA.h?rev=268471&view=auto
==============================================================================
--- cfe/trunk/test/CodeCompletion/Inputs/import_moduleA.h (added)
+++ cfe/trunk/test/CodeCompletion/Inputs/import_moduleA.h Tue May  3 19:53:13 2016
@@ -0,0 +1,2 @@
+#include "ModuleA/moduleA.h"
+static int const FROM_HEADER = 1;

Added: cfe/trunk/test/CodeCompletion/pch-and-module.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/pch-and-module.m?rev=268471&view=auto
==============================================================================
--- cfe/trunk/test/CodeCompletion/pch-and-module.m (added)
+++ cfe/trunk/test/CodeCompletion/pch-and-module.m Tue May  3 19:53:13 2016
@@ -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

Modified: cfe/trunk/test/Modules/Inputs/module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module.map?rev=268471&r1=268470&r2=268471&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/module.map (original)
+++ cfe/trunk/test/Modules/Inputs/module.map Tue May  3 19:53:13 2016
@@ -410,3 +410,5 @@ module MethodPoolString1 {
 module MethodPoolString2 {
   header "MethodPoolString2.h"
 }
+
+module Empty {}

Added: cfe/trunk/test/Modules/Inputs/typo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/typo.h?rev=268471&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/typo.h (added)
+++ cfe/trunk/test/Modules/Inputs/typo.h Tue May  3 19:53:13 2016
@@ -0,0 +1,6 @@
+ at import Empty;
+
+ at interface NSString
++ (id)alloc;
+ at end
+

Added: cfe/trunk/test/Modules/typo.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/typo.m?rev=268471&view=auto
==============================================================================
--- cfe/trunk/test/Modules/typo.m (added)
+++ cfe/trunk/test/Modules/typo.m Tue May  3 19:53:13 2016
@@ -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 at typo.h:* {{here}}
+}




More information about the cfe-commits mailing list