sammccall created this revision.
sammccall added reviewers: akyrtzi, arphaman.
Herald added subscribers: cfe-commits, ioeric, ilya-biryukov.

The FileID/Offset conversion is lossy. The code takes the fileLoc, which loses
e.g. the spelling location in some macro cases.
Instead, pass the original SourceLocation which preserves all information, and
update consumers to match current behavior.

This allows us to fix two bugs in clangd that need the spelling location.


Repository:
  rC Clang

https://reviews.llvm.org/D45014

Files:
  include/clang/Index/IndexDataConsumer.h
  lib/Index/IndexingAction.cpp
  lib/Index/IndexingContext.cpp
  tools/c-index-test/core_main.cpp
  tools/libclang/CXIndexDataConsumer.cpp
  tools/libclang/CXIndexDataConsumer.h

Index: tools/libclang/CXIndexDataConsumer.h
===================================================================
--- tools/libclang/CXIndexDataConsumer.h
+++ tools/libclang/CXIndexDataConsumer.h
@@ -465,12 +465,11 @@
 private:
   bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
                            ArrayRef<index::SymbolRelation> Relations,
-                           FileID FID, unsigned Offset,
-                           ASTNodeInfo ASTNode) override;
+                           SourceLocation Loc, ASTNodeInfo ASTNode) override;
 
   bool handleModuleOccurence(const ImportDecl *ImportD,
                              index::SymbolRoleSet Roles,
-                             FileID FID, unsigned Offset) override;
+                             SourceLocation Loc) override;
 
   void finish() override;
 
Index: tools/libclang/CXIndexDataConsumer.cpp
===================================================================
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -155,13 +155,10 @@
 }
 }
 
-bool CXIndexDataConsumer::handleDeclOccurence(const Decl *D,
-                                              SymbolRoleSet Roles,
-                                             ArrayRef<SymbolRelation> Relations,
-                                              FileID FID, unsigned Offset,
-                                              ASTNodeInfo ASTNode) {
-  SourceLocation Loc = getASTContext().getSourceManager()
-      .getLocForStartOfFile(FID).getLocWithOffset(Offset);
+bool CXIndexDataConsumer::handleDeclOccurence(
+    const Decl *D, SymbolRoleSet Roles, ArrayRef<SymbolRelation> Relations,
+    SourceLocation Loc, ASTNodeInfo ASTNode) {
+  Loc = getASTContext().getSourceManager().getFileLoc(Loc);
 
   if (Roles & (unsigned)SymbolRole::Reference) {
     const NamedDecl *ND = dyn_cast<NamedDecl>(D);
@@ -226,8 +223,7 @@
 
 bool CXIndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD,
                                                 SymbolRoleSet Roles,
-                                                FileID FID,
-                                                unsigned Offset) {
+                                                SourceLocation Loc) {
   IndexingDeclVisitor(*this, SourceLocation(), nullptr).Visit(ImportD);
   return !shouldAbort();
 }
Index: tools/c-index-test/core_main.cpp
===================================================================
--- tools/c-index-test/core_main.cpp
+++ tools/c-index-test/core_main.cpp
@@ -88,13 +88,14 @@
 
   bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
                            ArrayRef<SymbolRelation> Relations,
-                           FileID FID, unsigned Offset,
-                           ASTNodeInfo ASTNode) override {
+                           SourceLocation Loc, ASTNodeInfo ASTNode) override {
     ASTContext &Ctx = D->getASTContext();
     SourceManager &SM = Ctx.getSourceManager();
 
-    unsigned Line = SM.getLineNumber(FID, Offset);
-    unsigned Col = SM.getColumnNumber(FID, Offset);
+    Loc = SM.getFileLoc(Loc);
+    FileID FID = SM.getFileID(Loc);
+    unsigned Line = SM.getLineNumber(FID, SM.getFileOffset(Loc));
+    unsigned Col = SM.getColumnNumber(FID, SM.getFileOffset(Loc));
     OS << Line << ':' << Col << " | ";
 
     printSymbolInfo(getSymbolInfo(D), OS);
@@ -124,12 +125,14 @@
   }
 
   bool handleModuleOccurence(const ImportDecl *ImportD, SymbolRoleSet Roles,
-                             FileID FID, unsigned Offset) override {
+                             SourceLocation Loc) override {
     ASTContext &Ctx = ImportD->getASTContext();
     SourceManager &SM = Ctx.getSourceManager();
 
-    unsigned Line = SM.getLineNumber(FID, Offset);
-    unsigned Col = SM.getColumnNumber(FID, Offset);
+    Loc = SM.getFileLoc(Loc);
+    FileID FID = SM.getFileID(Loc);
+    unsigned Line = SM.getLineNumber(FID, SM.getFileOffset(Loc));
+    unsigned Col = SM.getColumnNumber(FID, SM.getFileOffset(Loc));
     OS << Line << ':' << Col << " | ";
 
     printSymbolInfo(getSymbolInfo(ImportD), OS);
Index: lib/Index/IndexingContext.cpp
===================================================================
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -82,14 +82,9 @@
     Loc = IdLocs.front();
   else
     Loc = ImportD->getLocation();
-  SourceManager &SM = Ctx->getSourceManager();
-  Loc = SM.getFileLoc(Loc);
-  if (Loc.isInvalid())
-    return true;
 
-  FileID FID;
-  unsigned Offset;
-  std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
+  SourceManager &SM = Ctx->getSourceManager();
+  FileID FID = SM.getFileID(SM.getFileLoc(Loc));
   if (FID.isInvalid())
     return true;
 
@@ -112,7 +107,7 @@
   if (ImportD->isImplicit())
     Roles |= (unsigned)SymbolRole::Implicit;
 
-  return DataConsumer.handleModuleOccurence(ImportD, Roles, FID, Offset);
+  return DataConsumer.handleModuleOccurence(ImportD, Roles, Loc);
 }
 
 bool IndexingContext::isTemplateImplicitInstantiation(const Decl *D) {
@@ -327,13 +322,7 @@
     return true;
 
   SourceManager &SM = Ctx->getSourceManager();
-  Loc = SM.getFileLoc(Loc);
-  if (Loc.isInvalid())
-    return true;
-
-  FileID FID;
-  unsigned Offset;
-  std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
+  FileID FID = SM.getFileID(SM.getFileLoc(Loc));
   if (FID.isInvalid())
     return true;
 
@@ -414,7 +403,6 @@
                                Rel.RelatedSymbol->getCanonicalDecl()));
   }
 
-  IndexDataConsumer::ASTNodeInfo Node{ OrigE, OrigD, Parent, ContainerDC };
-  return DataConsumer.handleDeclOccurence(D, Roles, FinalRelations, FID, Offset,
-                                          Node);
+  IndexDataConsumer::ASTNodeInfo Node{OrigE, OrigD, Parent, ContainerDC};
+  return DataConsumer.handleDeclOccurence(D, Roles, FinalRelations, Loc, Node);
 }
Index: lib/Index/IndexingAction.cpp
===================================================================
--- lib/Index/IndexingAction.cpp
+++ lib/Index/IndexingAction.cpp
@@ -23,20 +23,21 @@
 
 bool IndexDataConsumer::handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
                                             ArrayRef<SymbolRelation> Relations,
-                                            FileID FID, unsigned Offset,
+                                            SourceLocation Loc,
                                             ASTNodeInfo ASTNode) {
   return true;
 }
 
 bool IndexDataConsumer::handleMacroOccurence(const IdentifierInfo *Name,
-                                             const MacroInfo *MI, SymbolRoleSet Roles,
-                                             FileID FID, unsigned Offset) {
+                                             const MacroInfo *MI,
+                                             SymbolRoleSet Roles,
+                                             SourceLocation Loc) {
   return true;
 }
 
 bool IndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD,
                                               SymbolRoleSet Roles,
-                                              FileID FID, unsigned Offset) {
+                                              SourceLocation Loc) {
   return true;
 }
 
Index: include/clang/Index/IndexDataConsumer.h
===================================================================
--- include/clang/Index/IndexDataConsumer.h
+++ include/clang/Index/IndexDataConsumer.h
@@ -42,18 +42,16 @@
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
                                    ArrayRef<SymbolRelation> Relations,
-                                   FileID FID, unsigned Offset,
-                                   ASTNodeInfo ASTNode);
+                                   SourceLocation Loc, ASTNodeInfo ASTNode);
 
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleMacroOccurence(const IdentifierInfo *Name,
                                     const MacroInfo *MI, SymbolRoleSet Roles,
-                                    FileID FID, unsigned Offset);
+                                    SourceLocation Loc);
 
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleModuleOccurence(const ImportDecl *ImportD,
-                                     SymbolRoleSet Roles,
-                                     FileID FID, unsigned Offset);
+                                     SymbolRoleSet Roles, SourceLocation Loc);
 
   virtual void finish() {}
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to