[PATCH] D75259: [clangd] Get rid of unnecssary source transformations in locateMacroAt

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: sammccall, hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

All callers are already passing spelling locations to locateMacroAt.
Also there's no point at looking at macro expansion for figuring out undefs as
it is forbidden to have PP directives inside macro bodies.
Also fixes a bug when the previous sourcelocation is unavailable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75259

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp


Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -436,6 +436,20 @@
   EXPECT_THAT(*Result, MacroName("MACRO"));
 }
 
+TEST(SourceCodeTests, WorksAtBeginOfFile) {
+  Annotations Code("^MACRO;");
+  TestTU TU = TestTU::withCode(Code.code());
+  TU.HeaderCode = "#define MACRO int x;";
+  auto AST = TU.build();
+  auto CurLoc = sourceLocationInMainFile(AST.getSourceManager(), Code.point());
+  ASSERT_TRUE(bool(CurLoc));
+  const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
+  ASSERT_TRUE(Id);
+  auto Result = locateMacroAt(Id->location(), AST.getPreprocessor());
+  ASSERT_TRUE(Result);
+  EXPECT_THAT(*Result, MacroName("MACRO"));
+}
+
 TEST(SourceCodeTests, IsInsideMainFile){
   TestTU TU;
   TU.HeaderCode = R"cpp(
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -281,7 +281,8 @@
   llvm::StringRef Name;
   const MacroInfo *Info;
 };
-/// Gets the macro at a specified \p Loc.
+/// Gets the macro at a specified \p Loc. It must be a spelling location and
+/// point to the beginning of identifier.
 llvm::Optional<DefinedMacro> locateMacroAt(SourceLocation Loc,
                                            Preprocessor &PP);
 
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -896,10 +896,11 @@
 
 llvm::Optional<DefinedMacro> locateMacroAt(SourceLocation Loc,
                                            Preprocessor &PP) {
+  assert(Loc.isFileID());
   const auto &SM = PP.getSourceManager();
   const auto &LangOpts = PP.getLangOpts();
   Token Result;
-  if (Lexer::getRawToken(SM.getSpellingLoc(Loc), Result, SM, LangOpts, false))
+  if (Lexer::getRawToken(Loc, Result, SM, LangOpts, false))
     return None;
   if (Result.is(tok::raw_identifier))
     PP.LookUpIdentifierInfo(Result);
@@ -907,14 +908,12 @@
   if (!IdentifierInfo || !IdentifierInfo->hadMacroDefinition())
     return None;
 
-  std::pair<FileID, unsigned int> DecLoc = SM.getDecomposedExpansionLoc(Loc);
   // Get the definition just before the searched location so that a macro
-  // referenced in a '#undef MACRO' can still be found.
-  SourceLocation BeforeSearchedLocation =
-      SM.getMacroArgExpandedLocation(SM.getLocForStartOfFile(DecLoc.first)
-                                         .getLocWithOffset(DecLoc.second - 1));
-  MacroDefinition MacroDef =
-      PP.getMacroDefinitionAtLoc(IdentifierInfo, BeforeSearchedLocation);
+  // referenced in a '#undef MACRO' can still be found. Note that we only do
+  // that if Loc is not pointing at start of file.
+  if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
+    Loc = Loc.getLocWithOffset(-1);
+  MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
   if (auto *MI = MacroDef.getMacroInfo())
     return DefinedMacro{IdentifierInfo->getName(), MI};
   return None;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75259.246948.patch
Type: text/x-patch
Size: 3280 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200227/c52ce933/attachment.bin>