[clang-tools-extra] c24c89d - [clangd] Get rid of unnecessary source transformations in locateMacroAt

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-03-02T13:31:12+01:00
New Revision: c24c89d6f0f55bb95995b4f5587c791ac8d738fc

URL: https://github.com/llvm/llvm-project/commit/c24c89d6f0f55bb95995b4f5587c791ac8d738fc
DIFF: https://github.com/llvm/llvm-project/commit/c24c89d6f0f55bb95995b4f5587c791ac8d738fc.diff

LOG: [clangd] Get rid of unnecessary source transformations in locateMacroAt

Summary:
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.

Reviewers: sammccall, hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 3feddd1df24b..2c4338dce7c7 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -893,10 +893,11 @@ llvm::StringSet<> collectWords(llvm::StringRef Content) {
 
 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);
@@ -904,14 +905,12 @@ llvm::Optional<DefinedMacro> locateMacroAt(SourceLocation Loc,
   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;

diff  --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index 137a6f664ed9..32fe9c6c54b3 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -281,7 +281,8 @@ struct DefinedMacro {
   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);
 

diff  --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
index 305e07d7c0c3..7d614785a11d 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -436,6 +436,20 @@ TEST(SourceCodeTests, GetMacros) {
   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(