Show inclusions from a preamble in clang_getInclusions.

[erikjv]

Summary

When reparsing a translation unit with preamble generation turned on,
no includes are found. This is due to the fact that all SLocs from
AST/PCH files are skipped as they are 'loaded', and inclusions from a
preamble are also 'loaded'. So, in case a file has a preamble, it first
needs to process those loaded inclusions, and then check for any local
inclusions. This latter one is for any includes that are not part of the
preamble, like includes half-way through a file.

[skalinichev]

I've seen this bug too. Generally this patch looks good to me.

Could you also add a test for includes half-way through a file case?

tools/c-index-test/c-index-test.c

1567 ↗

(On Diff #39187)

This is not a very good idea to add this code here IMO. There are probably some tests that use this function with CINDEXTEST_EDITING set, which could potentially hide some parsing vs. reparsing bugs. I'd suggest to use something like perform_test_reparse_source instead.

[bkramer]

LGTM, thanks for the fix!

tools/libclang/CIndexInclusionStack.cpp

32 ↗

(On Diff #39187)

There's already a wild mix of coding styles in this file, let's not add more of them. bool HasPreamble is the preferred style.

cfe/trunk/test/Index/cindex-test-inclusions.c

 // RUN: c-index-test -test-inclusion-stack-source %s 2>&1 | FileCheck %s
 
 #include "include_test.h"
 
 // CHECK: cindex-test-inclusions.c
 // CHECK: included by:
 // CHECK: include_test.h
 // CHECK: included by:
 // CHECK: cindex-test-inclusions.c:3:10
 // CHECK: include_test_2.h
 // CHECK: included by:
 // CHECK: include_test.h:1:10
 // CHECK: cindex-test-inclusions.c:3:10
+
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-inclusion-stack-source %s 2>&1 | FileCheck -check-prefix=REPARSE %s
+// REPARSE: include_test_2.h
+// REPARSE: included by:
+// REPARSE: include_test.h:1:10
+// REPARSE: cindex-test-inclusions.c:3:10
+// REPARSE: include_test.h
+// REPARSE: included by:
+// REPARSE: cindex-test-inclusions.c:3:10
+// REPARSE: cindex-test-inclusions.c
+// REPARSE: included by:

cfe/trunk/tools/c-index-test/c-index-test.c

int perform_test_load_source(int argc, const char **argv,
                              PostVisitTU PV) {
   CXIndex Idx;
   CXTranslationUnit TU;
   const char *CommentSchemaFile;
   struct CXUnsavedFile *unsaved_files = 0;
   int num_unsaved_files = 0;
   enum CXErrorCode Err;
   int result;
+  unsigned Repeats = 0;
+  unsigned I;
 
   Idx = clang_createIndex(/* excludeDeclsFromPCH */
                           (!strcmp(filter, "local") || 
                            !strcmp(filter, "local-display"))? 1 : 0,
                           /* displayDiagnostics=*/1);
 
   if ((CommentSchemaFile = parse_comments_schema(argc, argv))) {
     argc--;
     argv++;
   }
 
   if (parse_remapped_files(argc, argv, 0, &unsaved_files, &num_unsaved_files)) {
     clang_disposeIndex(Idx);
     return -1;
   }
 
+  if (getenv("CINDEXTEST_EDITING"))
+    Repeats = 5;
+
   Err = clang_parseTranslationUnit2(Idx, 0,
                                     argv + num_unsaved_files,
                                     argc - num_unsaved_files,
                                     unsaved_files, num_unsaved_files,
                                     getDefaultParsingOptions(), &TU);
   if (Err != CXError_Success) {
     fprintf(stderr, "Unable to load translation unit!\n");
     describeLibclangFailure(Err);
     free_remapped_files(unsaved_files, num_unsaved_files);
     clang_disposeIndex(Idx);
     return 1;
   }
 
+  for (I = 0; I != Repeats; ++I) {
+    if (checkForErrors(TU) != 0)
+      return -1;
+
+    if (Repeats > 1) {
+      Err = clang_reparseTranslationUnit(TU, num_unsaved_files, unsaved_files,
+                                         clang_defaultReparseOptions(TU));
+      if (Err != CXError_Success) {
+        describeLibclangFailure(Err);
+        free_remapped_files(unsaved_files, num_unsaved_files);
+        clang_disposeIndex(Idx);
+        return 1;
+      }
+    }
+  }
+
   result = perform_test_load(Idx, TU, filter, NULL, Visitor, PV,
                              CommentSchemaFile);
   free_remapped_files(unsaved_files, num_unsaved_files);
   clang_disposeIndex(Idx);
   return result;
 }
 
 int perform_test_reparse_source(int argc, const char **argv, int trials,

cfe/trunk/tools/libclang/CIndexInclusionStack.cpp

 #include "CXSourceLocation.h"
 #include "CXTranslationUnit.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 
-extern "C" {
-void clang_getInclusions(CXTranslationUnit TU, CXInclusionVisitor CB,
-                         CXClientData clientData) {
-  if (cxtu::isNotUsableTU(TU)) {
-    LOG_BAD_TU(TU);
-    return;
-  }
-
+static void getInclusions(const SrcMgr::SLocEntry &(SourceManager::*Getter)(unsigned, bool*) const, unsigned n,
+                          CXTranslationUnit TU, CXInclusionVisitor CB,
+                          CXClientData clientData)
+{
   ASTUnit *CXXUnit = cxtu::getASTUnit(TU);
   SourceManager &SM = CXXUnit->getSourceManager();
   ASTContext &Ctx = CXXUnit->getASTContext();
-
   SmallVector<CXSourceLocation, 10> InclusionStack;
-  unsigned n =  SM.local_sloc_entry_size();
-
-  // In the case where all the SLocEntries are in an external source, traverse
-  // those SLocEntries as well.  This is the case where we are looking
-  // at the inclusion stack of an AST/PCH file.
-  const SrcMgr::SLocEntry &(SourceManager::*Getter)(unsigned, bool*) const;
-  if (n == 1) {
-    Getter = &SourceManager::getLoadedSLocEntry;
-    n = SM.loaded_sloc_entry_size();
-  } else
-    Getter = &SourceManager::getLocalSLocEntry;
+  const bool HasPreamble = SM.getPreambleFileID().isValid();
 
   for (unsigned i = 0 ; i < n ; ++i) {
     bool Invalid = false;
     const SrcMgr::SLocEntry &SL = (SM.*Getter)(i, &Invalid);
-    
     if (!SL.isFile() || Invalid)
       continue;
 
     const SrcMgr::FileInfo &FI = SL.getFile();
     if (!FI.getContentCache()->OrigEntry)
       continue;
-    
-    // Build the inclusion stack.
+    // If this is the main file, and there is a preamble, skip this SLoc. The
+    // inclusions of the preamble already showed it.
     SourceLocation L = FI.getIncludeLoc();
+    if (HasPreamble && CXXUnit->isInMainFileID(L))
+      continue;
+
+    // Build the inclusion stack.
     InclusionStack.clear();
     while (L.isValid()) {
       PresumedLoc PLoc = SM.getPresumedLoc(L);
       InclusionStack.push_back(cxloc::translateSourceLocation(Ctx, L));
       L = PLoc.isValid()? PLoc.getIncludeLoc() : SourceLocation();
     }
-            
+    // If there is a preamble, the last entry is the "inclusion" of that
+    // preamble into the main file, which has the bogus entry of main.c:1:1
+    if (HasPreamble && !InclusionStack.empty())
+      InclusionStack.pop_back();
+
     // Callback to the client.
     // FIXME: We should have a function to construct CXFiles.
     CB(static_cast<CXFile>(
-         const_cast<FileEntry *>(FI.getContentCache()->OrigEntry)), 
+         const_cast<FileEntry *>(FI.getContentCache()->OrigEntry)),
        InclusionStack.data(), InclusionStack.size(), clientData);
-  }    
+  }
 }
+
+
+extern "C" {
+void clang_getInclusions(CXTranslationUnit TU, CXInclusionVisitor CB,
+                         CXClientData clientData) {
+  if (cxtu::isNotUsableTU(TU)) {
+    LOG_BAD_TU(TU);
+    return;
+  }
+
+  SourceManager &SM = cxtu::getASTUnit(TU)->getSourceManager();
+  const unsigned n =  SM.local_sloc_entry_size();
+
+  // In the case where all the SLocEntries are in an external source, traverse
+  // those SLocEntries as well.  This is the case where we are looking
+  // at the inclusion stack of an AST/PCH file. Also, if we are not looking at
+  // a AST/PCH file, but this file has a pre-compiled preamble, we also need
+  // to look in that file.
+  if (n == 1 || SM.getPreambleFileID().isValid()) {
+    getInclusions(&SourceManager::getLoadedSLocEntry,
+                  SM.loaded_sloc_entry_size(), TU, CB, clientData);
+  }
+
+  // Not a PCH/AST file. Note, if there is a preamble, it could still be that
+  // there are #includes in this file (e.g. for any include after the first
+  // declaration).
+  if (n != 1)
+    getInclusions(&SourceManager::getLocalSLocEntry, n, TU, CB, clientData);
+
+}
 } // end extern C