[clangd] Store offsets in MacroOccurrence

Remove the existing `Rng` field.

From the review comment: https://reviews.llvm.org/D147034

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D153259

diff --git a/clang-tools-extra/clangd/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp
index 9fa3d69..c5ba8d9 100644 (file)
--- a/clang-tools-extra/clangd/CollectMacros.cpp
+++ b/clang-tools-extra/clangd/CollectMacros.cpp
@@ -8,12 +8,22 @@
 
 #include "CollectMacros.h"
 #include "AST.h"
+#include "Protocol.h"
+#include "SourceCode.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/STLExtras.h"
+#include <cstddef>
 
 namespace clang {
 namespace clangd {
 
+Range MacroOccurrence::toRange(const SourceManager &SM) const {
+  auto MainFile = SM.getMainFileID();
+  return halfOpenToRange(
+      SM, syntax::FileRange(MainFile, StartOffset, EndOffset).toCharRange(SM));
+}
+
 void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI,
                                 bool IsDefinition, bool InIfCondition) {
   if (!InMainFile)
@@ -24,12 +34,12 @@ void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI,
 
   auto Name = MacroNameTok.getIdentifierInfo()->getName();
   Out.Names.insert(Name);
-  auto Range = halfOpenToRange(
-      SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc()));
+  size_t Start = SM.getFileOffset(Loc);
+  size_t End = SM.getFileOffset(MacroNameTok.getEndLoc());
   if (auto SID = getSymbolID(Name, MI, SM))
-    Out.MacroRefs[SID].push_back({Range, IsDefinition, InIfCondition});
+    Out.MacroRefs[SID].push_back({Start, End, IsDefinition, InIfCondition});
   else
-    Out.UnknownMacros.push_back({Range, IsDefinition, InIfCondition});
+    Out.UnknownMacros.push_back({Start, End, IsDefinition, InIfCondition});
 }
 
 void CollectMainFileMacros::FileChanged(SourceLocation Loc, FileChangeReason,

diff --git a/clang-tools-extra/clangd/CollectMacros.h b/clang-tools-extra/clangd/CollectMacros.h
index 716a5d5..e3900c0 100644 (file)
--- a/clang-tools-extra/clangd/CollectMacros.h
+++ b/clang-tools-extra/clangd/CollectMacros.h
@@ -16,18 +16,22 @@
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/DenseMap.h"
+#include <cstddef>
 #include <string>
 
 namespace clang {
 namespace clangd {
 
 struct MacroOccurrence {
-  // Instead of storing SourceLocation, we have to store the token range because
-  // SourceManager from preamble is not available when we build the AST.
-  Range Rng;
+  // Half-open range (end offset is exclusive) inside the main file.
+  size_t StartOffset;
+  size_t EndOffset;
+
   bool IsDefinition;
   // True if the occurence is used in a conditional directive, e.g. #ifdef MACRO
   bool InConditionalDirective;
+
+  Range toRange(const SourceManager &SM) const;
 };
 
 struct MainFileMacros {

diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 6a835f3..3826170 100644 (file)
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -1211,7 +1211,8 @@ getSemanticHighlightings(ParsedAST &AST, bool IncludeInactiveRegionTokens) {
       AST.getHeuristicResolver());
   // Add highlightings for macro references.
   auto AddMacro = [&](const MacroOccurrence &M) {
-    auto &T = Builder.addToken(M.Rng, HighlightingKind::Macro);
+    auto &T = Builder.addToken(M.toRange(C.getSourceManager()),
+                               HighlightingKind::Macro);
     T.addModifier(HighlightingModifier::GlobalScope);
     if (M.IsDefinition)
       T.addModifier(HighlightingModifier::Declaration);

diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index ad4819f..b608296 100644 (file)
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -1408,7 +1408,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
       if (Refs != IDToRefs.end()) {
         for (const auto &Ref : Refs->second) {
           ReferencesResult::Reference Result;
-          Result.Loc.range = Ref.Rng;
+          Result.Loc.range = Ref.toRange(SM);
           Result.Loc.uri = URIMainFile;
           if (Ref.IsDefinition) {
             Result.Attributes |= ReferencesResult::Declaration;

diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 131d0a3..5e19f1b 100644 (file)
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -642,7 +642,7 @@ void SymbolCollector::handleMacros(const MainFileMacros &MacroRefsToIndex) {
   // Add macro references.
   for (const auto &IDToRefs : MacroRefsToIndex.MacroRefs) {
     for (const auto &MacroRef : IDToRefs.second) {
-      const auto &Range = MacroRef.Rng;
+      const auto &Range = MacroRef.toRange(SM);
       bool IsDefinition = MacroRef.IsDefinition;
       Ref R;
       R.Location.Start.setLine(Range.start.line);

diff --git a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
index 459d18a..91b2d3e 100644 (file)
--- a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -23,7 +23,9 @@ namespace {
 
 using testing::UnorderedElementsAreArray;
 
-MATCHER_P(rangeIs, R, "") { return arg.Rng == R; }
+MATCHER_P(rangeIs, R, "") {
+  return arg.StartOffset == R.Begin && arg.EndOffset == R.End;
+}
 MATCHER(isDef, "") { return arg.IsDefinition; }
 MATCHER(inConditionalDirective, "") { return arg.InConditionalDirective; }
 
@@ -90,7 +92,7 @@ TEST(CollectMainFileMacros, SelectedMacros) {
         #define $2(def)[[FOO]] $3[[BAR]]
         int A = $2[[FOO]];
       )cpp"};
-  auto ExpectedResults = [](const Annotations &T, StringRef Name) {
+  auto ExpectedResults = [](const llvm::Annotations &T, StringRef Name) {
     std::vector<Matcher<MacroOccurrence>> ExpectedLocations;
     for (const auto &[R, Bits] : T.rangesWithPayload(Name)) {
       if (Bits == "def")
@@ -105,7 +107,7 @@ TEST(CollectMainFileMacros, SelectedMacros) {
   };
 
   for (const char *Test : Tests) {
-    Annotations T(Test);
+    llvm::Annotations T(Test);
     auto Inputs = TestTU::withCode(T.code());
     Inputs.ExtraArgs.push_back("-std=c++2b");
     auto AST = Inputs.build();

diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 452d330..09986b4 100644 (file)
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -13,7 +13,6 @@
 
 #include "../../clang-tidy/ClangTidyCheck.h"
 #include "AST.h"
-#include "Annotations.h"
 #include "Compiler.h"
 #include "Config.h"
 #include "Diagnostics.h"
@@ -31,6 +30,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Annotations/Annotations.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock-matchers.h"
 #include "gmock/gmock.h"
@@ -281,7 +281,7 @@ TEST(ParsedASTTest, CanBuildInvocationWithUnknownArgs) {
 }
 
 TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
-  Annotations TestCase(R"cpp(
+  llvm::Annotations TestCase(R"cpp(
     #define ^MACRO_ARGS(X, Y) X Y
     // - preamble ends
     ^ID(int A);
@@ -334,15 +334,16 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
     int D = DEF;
   )cpp";
   ParsedAST AST = TU.build();
-  std::vector<Position> MacroExpansionPositions;
+  std::vector<size_t> MacroExpansionPositions;
   for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
     for (const auto &R : SIDToRefs.second)
-      MacroExpansionPositions.push_back(R.Rng.start);
+      MacroExpansionPositions.push_back(R.StartOffset);
   }
   for (const auto &R : AST.getMacros().UnknownMacros)
-    MacroExpansionPositions.push_back(R.Rng.start);
-  EXPECT_THAT(MacroExpansionPositions,
-              testing::UnorderedElementsAreArray(TestCase.points()));
+    MacroExpansionPositions.push_back(R.StartOffset);
+  EXPECT_THAT(
+      MacroExpansionPositions,
+      testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
 MATCHER_P(withFileName, Inc, "") { return arg.FileName == Inc; }

diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index 4f2cc3e..a370fa8 100644 (file)
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -30,6 +30,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest-matchers.h"
 #include "gtest/gtest.h"
@@ -322,16 +323,16 @@ TEST(PreamblePatchTest, Define) {
 
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Contents);
-    Annotations Modified(Case.Contents);
+    llvm::Annotations Modified(Case.Contents);
     EXPECT_THAT(getPreamblePatch("", Modified.code()),
                 MatchesRegex(Case.ExpectedPatch));
 
     auto AST = createPatchedAST("", Modified.code());
     ASSERT_TRUE(AST);
-    std::vector<Range> MacroRefRanges;
+    std::vector<llvm::Annotations::Range> MacroRefRanges;
     for (auto &M : AST->getMacros().MacroRefs) {
       for (auto &O : M.getSecond())
-        MacroRefRanges.push_back(O.Rng);
+        MacroRefRanges.push_back({O.StartOffset, O.EndOffset});
     }
     EXPECT_THAT(MacroRefRanges, Contains(Modified.range()));
   }