[clangd] Collect macros in the preamble region of the main file

[hokein]

Summary

• store all macro references in the ParsedAST, which also helps to implement find references for macros.
• unify the two variants of CollectMainFileMacros.

[hokein]

some cleanup

[hokein]

friendly ping, in case you missing it :)

[ilya-biryukov]

Sorry for the delay

clang-tools-extra/clangd/Macro.h

59 ↗

(On Diff #219904)

Converting here is too early, could we keep this conversion in syntax highlighting code?
Keeping source locations here is enough.

67 ↗

(On Diff #219904)

Could we model in a way that avoids duplicating macro names on each occurrence?
We had StringSet Names and vector<SourceLocation> Locations, let's keep it in the same way.

We could group this into a struct to reduce boilerplate of transferring it around, obviously

struct MainFileMacros {
  StringSet Names;
  vector<SourceLocation> Locations;
};

[hokein]

clang-tools-extra/clangd/Macro.h

59 ↗

(On Diff #219904)

I'm not sure this is doable -- to get a range, we need the SourceManager, in the syntax highlighting context, we get the SourceManager from the ParsedAST, this SourceManager is used when building the main AST with preamble, I don't think we can use this SourceManager to get the token range for macros in the preamble section?

67 ↗

(On Diff #219904)

yes, we don't need the name and location reletionship in this patch, but we'd need this when implementing xrefs for macros. do you think we should keep the same way, and do the change when we start implementing xrefs for macros?

[ilya-biryukov]

clang-tools-extra/clangd/Macro.h

59 ↗

(On Diff #219904)

Ah, yeah, good point. The source manager from preamble is not there anymore.
Ranges LG and this is what we used before.

Thanks for clearing this up.

67 ↗

(On Diff #219904)

Cross-references have to be more fine-grained and having only a name is not enough. Let's address this separately when/if we actually need this in xrefs.
In particular, we want to handle macro re-definitions:

#define FOO 123 // #1
int a = FOO; // a reference to #1

#define FOO 234 // #2
int b = FOO; // a reference to #2

Current model will merge these two together as both names are the same.

[hokein]

Address comments.

[hokein]

clang-tools-extra/clangd/Macro.h

67 ↗

(On Diff #219904)

good point, agreed. I think we probably use the symbol id for macros, but it is out-scope of this patch.

[ilya-biryukov]

LGTM, thanks.

A ton of small NITs too, but they're all small details.

clang-tools-extra/clangd/Macro.h

1 ↗

(On Diff #221492)

NIT: Maybe rename to CollectMacros.h?

Macro.h looks a bit too generic and may start being a place for more helpers. However, the current file is pretty well-structured.

23 ↗

(On Diff #221492)

Could you add a comment that we have to convert to Range early because source manager from preamble is not available when we build the AST?

26 ↗

(On Diff #221492)

NIT: a typo: in the main file. It is used ...

28 ↗

(On Diff #221492)

NIT: use /// to enable doxygen comments

65 ↗

(On Diff #221492)

NIT: It's probably safer to set 'no' by default.
Normally the first thing visited by the parser is <builtin> buffer, not the main file.

OTOH, FileChanged should be called first in any case, so it does not matter.

clang-tools-extra/clangd/ParsedAST.h

104 ↗

(On Diff #221492)

NIT: rename parameter to Macros

124 ↗

(On Diff #221492)

NIT: change definitions/expansions to definitions and expansion

clang-tools-extra/clangd/Preamble.cpp

34 ↗

(On Diff #221492)

NIT: rename to takeMacros

73 ↗

(On Diff #221492)

NIT: add const to SourceManager too, for consistency
(if possible)

clang-tools-extra/clangd/Preamble.h

61 ↗

(On Diff #221492)

There seems to be no reason for moving constructor to the bottom of the class. Maybe keep as is?

[hokein]

Address review comments.

[hokein]

update header guard.

clang-tools-extra/trunk/clangd/CodeComplete.cpp

void loadMainFilePreambleMacros(const Preprocessor &PP,
   // but this includes transitively included files, so may deserialize a lot.
   ExternalPreprocessorSource *PreambleMacros = PP.getExternalSource();
   // As we have the names of the macros, we can look up their IdentifierInfo
   // and then use this to load just the macros we want.
   IdentifierInfoLookup *PreambleIdentifiers =
       PP.getIdentifierTable().getExternalIdentifierLookup();
   if (!PreambleIdentifiers || !PreambleMacros)
     return;
-  for (const auto &MacroName : Preamble.MainFileMacros)
-    if (auto *II = PreambleIdentifiers->get(MacroName))
+  for (const auto &MacroName : Preamble.Macros.Names)
+    if (auto *II = PreambleIdentifiers->get(MacroName.getKey()))
       if (II->isOutOfDate())
         PreambleMacros->updateOutOfDateIdentifier(*II);
 }
 
 // Invokes Sema code completion on a file.
 // If \p Includes is set, it will be updated based on the compiler invocation.
 bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
                       const clang::CodeCompleteOptions &Options,

clang-tools-extra/trunk/clangd/CollectMacros.h

+//===--- CollectMacros.h -----------------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
+
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Lex/PPCallbacks.h"
+#include <string>
+
+namespace clang {
+namespace clangd {
+
+struct MainFileMacros {
+  llvm::StringSet<> Names;
+  // Instead of storing SourceLocation, we have to store the token range because
+  // SourceManager from preamble is not available when we build the AST.
+  std::vector<Range> Ranges;
+};
+
+/// Collects macro definitions and expansions in the main file. It is used to:
+///  - collect macros in the preamble section of the main file (in Preamble.cpp)
+///  - collect macros after the preamble of the main file (in ParsedAST.cpp)
+class CollectMainFileMacros : public PPCallbacks {
+public:
+  explicit CollectMainFileMacros(const SourceManager &SM,
+                                 const LangOptions &LangOpts,
+                                 MainFileMacros &Out)
+      : SM(SM), LangOpts(LangOpts), Out(Out) {}
+
+  void FileChanged(SourceLocation Loc, FileChangeReason,
+                   SrcMgr::CharacteristicKind, FileID) override {
+    InMainFile = isInsideMainFile(Loc, SM);
+  }
+
+  void MacroDefined(const Token &MacroName, const MacroDirective *MD) override {
+    add(MacroName, MD->getMacroInfo());
+  }
+
+  void MacroExpands(const Token &MacroName, const MacroDefinition &MD,
+                    SourceRange Range, const MacroArgs *Args) override {
+    add(MacroName, MD.getMacroInfo());
+  }
+
+private:
+  void add(const Token &MacroNameTok, const MacroInfo *MI) {
+    if (!InMainFile)
+      return;
+    auto Loc = MacroNameTok.getLocation();
+    if (Loc.isMacroID())
+      return;
+
+    if (auto Range = getTokenRange(SM, LangOpts, MacroNameTok.getLocation())) {
+      Out.Names.insert(MacroNameTok.getIdentifierInfo()->getName());
+      Out.Ranges.push_back(*Range);
+    }
+  }
+  const SourceManager &SM;
+  const LangOptions &LangOpts;
+  bool InMainFile = true;
+  MainFileMacros &Out;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H

clang-tools-extra/trunk/clangd/ParsedAST.h

 //  - running clang-tidy checks checks
 //
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PARSEDAST_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PARSEDAST_H
 
+#include "CollectMacros.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "Path.h"
 #include "Preamble.h"
 #include "index/CanonicalIncludes.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
public:
   const std::vector<Diag> &getDiagnostics() const;
 
   /// Returns the esitmated size of the AST and the accessory structures, in
   /// bytes. Does not include the size of the preamble.
   std::size_t getUsedBytes() const;
   const IncludeStructure &getIncludeStructure() const;
   const CanonicalIncludes &getCanonicalIncludes() const;
 
-  /// Gets all macro locations (definition, expansions) present in the main
-  /// file.
-  /// NOTE: macros inside the preamble are not included.
-  llvm::ArrayRef<SourceLocation> getMacros() const;
+  /// Gets all macro references (definition, expansions) present in the main
+  /// file, including those in the preamble region.
+  const MainFileMacros &getMacros() const;
   /// Tokens recorded while parsing the main file.
   /// (!) does not have tokens from the preamble.
   const syntax::TokenBuffer &getTokens() const { return Tokens; }
 
 private:
   ParsedAST(std::shared_ptr<const PreambleData> Preamble,
             std::unique_ptr<CompilerInstance> Clang,
             std::unique_ptr<FrontendAction> Action, syntax::TokenBuffer Tokens,
-            std::vector<SourceLocation> MainFileMacroExpLocs,
-            std::vector<Decl *> LocalTopLevelDecls, std::vector<Diag> Diags,
-            IncludeStructure Includes, CanonicalIncludes CanonIncludes);
+            MainFileMacros Macros, std::vector<Decl *> LocalTopLevelDecls,
+            std::vector<Diag> Diags, IncludeStructure Includes,
+            CanonicalIncludes CanonIncludes);
 
   // In-memory preambles must outlive the AST, it is important that this member
   // goes before Clang and Action.
   std::shared_ptr<const PreambleData> Preamble;
   // We store an "incomplete" FrontendAction (i.e. no EndSourceFile was called
   // on it) and CompilerInstance used to run it. That way we don't have to do
   // complex memory management of all Clang structures on our own. (They are
   // stored in CompilerInstance and cleaned up by
   // FrontendAction.EndSourceFile).
   std::unique_ptr<CompilerInstance> Clang;
   std::unique_ptr<FrontendAction> Action;
   /// Tokens recorded after the preamble finished.
   ///   - Includes all spelled tokens for the main file.
   ///   - Includes expanded tokens produced **after** preabmle.
   ///   - Does not have spelled or expanded tokens for files from preamble.
   syntax::TokenBuffer Tokens;
 
-  /// The start locations of all macro definitions/expansions spelled **after**
-  /// preamble.
-  /// Does not include locations from inside other macro expansions.
-  std::vector<SourceLocation> MacroIdentifierLocs;
+  /// All macro definitions and expansions in the main file.
+  MainFileMacros Macros;
   // Data, stored after parsing.
   std::vector<Diag> Diags;
   // Top-level decls inside the current file. Not that this does not include
   // top-level decls from the preamble.
   std::vector<Decl *> LocalTopLevelDecls;
   IncludeStructure Includes;
   CanonicalIncludes CanonIncludes;
 };

clang-tools-extra/trunk/clangd/ParsedAST.cpp

protected:
   CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
     return std::make_unique<DeclTrackingASTConsumer>(/*ref*/ TopLevelDecls);
   }
 
 private:
   std::vector<Decl *> TopLevelDecls;
 };
 
-// This collects macro expansions/definitions in the main file.
-// (Contrast with CollectMainFileMacros in Preamble.cpp, which collects macro
-// *definitions* in the preamble region of the main file).
-class CollectMainFileMacros : public PPCallbacks {
-  const SourceManager &SM;
-  std::vector<SourceLocation> &MainFileMacroLocs;
-
-  void addLoc(SourceLocation Loc) {
-    if (!Loc.isMacroID() && isInsideMainFile(Loc, SM))
-      MainFileMacroLocs.push_back(Loc);
-  }
-
-public:
-  CollectMainFileMacros(const SourceManager &SM,
-                        std::vector<SourceLocation> &MainFileMacroLocs)
-      : SM(SM), MainFileMacroLocs(MainFileMacroLocs) {}
-
-  void MacroDefined(const Token &MacroNameTok,
-                    const MacroDirective *MD) override {
-    addLoc(MacroNameTok.getLocation());
-  }
-  void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
-                    SourceRange Range, const MacroArgs *Args) override {
-    addLoc(MacroNameTok.getLocation());
-  }
-};
-
 // When using a preamble, only preprocessor events outside its bounds are seen.
 // This is almost what we want: replaying transitive preprocessing wastes time.
 // However this confuses clang-tidy checks: they don't see any #includes!
 // So we replay the *non-transitive* #includes that appear in the main-file.
 // It would be nice to replay other events (macro definitions, ifdefs etc) but
 // this addresses the most common cases fairly cheaply.
 class ReplayPreamble : private PPCallbacks {
 public:
ParsedAST::build(std::unique_ptr<clang::CompilerInvocation> CI,
   // Replay the preamble includes so that clang-tidy checks can see them.
   if (Preamble)
     ReplayPreamble::attach(Includes, *Clang);
   // Important: collectIncludeStructure is registered *after* ReplayPreamble!
   // Otherwise we would collect the replayed includes again...
   // (We can't *just* use the replayed includes, they don't have Resolved path).
   Clang->getPreprocessor().addPPCallbacks(
       collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
-  // Collect the macro expansions in the main file.
-  std::vector<SourceLocation> MainFileMacroExpLocs;
+  // Copy over the macros in the preamble region of the main file, and combine
+  // with non-preamble macros below.
+  MainFileMacros Macros;
+  if (Preamble)
+    Macros = Preamble->Macros;
   Clang->getPreprocessor().addPPCallbacks(
       std::make_unique<CollectMainFileMacros>(Clang->getSourceManager(),
-                                              MainFileMacroExpLocs));
+                                              Clang->getLangOpts(), Macros));
 
   // Copy over the includes from the preamble, then combine with the
   // non-preamble includes below.
   CanonicalIncludes CanonIncludes;
   if (Preamble)
     CanonIncludes = Preamble->CanonIncludes;
   else
     CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts());
ParsedAST::build(std::unique_ptr<clang::CompilerInvocation> CI,
   if (Preamble)
     Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end());
   // Finally, add diagnostics coming from the AST.
   {
     std::vector<Diag> D = ASTDiags.take(CTContext.getPointer());
     Diags.insert(Diags.end(), D.begin(), D.end());
   }
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-                   std::move(Tokens), std::move(MainFileMacroExpLocs),
-                   std::move(ParsedDecls), std::move(Diags),
-                   std::move(Includes), std::move(CanonIncludes));
+                   std::move(Tokens), std::move(Macros), std::move(ParsedDecls),
+                   std::move(Diags), std::move(Includes),
+                   std::move(CanonIncludes));
 }
 
 ParsedAST::ParsedAST(ParsedAST &&Other) = default;
 
 ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default;
 
 ParsedAST::~ParsedAST() {
   if (Action) {

 const Preprocessor &ParsedAST::getPreprocessor() const {
   return Clang->getPreprocessor();
 }
 
 llvm::ArrayRef<Decl *> ParsedAST::getLocalTopLevelDecls() {
   return LocalTopLevelDecls;
 }
 
-llvm::ArrayRef<SourceLocation> ParsedAST::getMacros() const {
-  return MacroIdentifierLocs;
-}
+const MainFileMacros &ParsedAST::getMacros() const { return Macros; }
 
 const std::vector<Diag> &ParsedAST::getDiagnostics() const { return Diags; }
 
 std::size_t ParsedAST::getUsedBytes() const {
   auto &AST = getASTContext();
   // FIXME(ibiryukov): we do not account for the dynamically allocated part of
   // Message and Fixes inside each diagnostic.
   std::size_t Total =

 
 const CanonicalIncludes &ParsedAST::getCanonicalIncludes() const {
   return CanonIncludes;
 }
 
 ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble,
                      std::unique_ptr<CompilerInstance> Clang,
                      std::unique_ptr<FrontendAction> Action,
-                     syntax::TokenBuffer Tokens,
-                     std::vector<SourceLocation> MacroIdentifierLocs,
+                     syntax::TokenBuffer Tokens, MainFileMacros Macros,
                      std::vector<Decl *> LocalTopLevelDecls,
                      std::vector<Diag> Diags, IncludeStructure Includes,
                      CanonicalIncludes CanonIncludes)
     : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
       Action(std::move(Action)), Tokens(std::move(Tokens)),
-      MacroIdentifierLocs(std::move(MacroIdentifierLocs)),
-      Diags(std::move(Diags)),
+      Macros(std::move(Macros)), Diags(std::move(Diags)),
       LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
       Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {
   assert(this->Clang);
   assert(this->Action);
 }
 
 llvm::Optional<ParsedAST>
 buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,

clang-tools-extra/trunk/clangd/Preamble.h

 // code-completion to have sub-second latency. It is supported by the
 // PrecompiledPreamble functionality in clang, which wraps the techniques used
 // by PCH files, modules etc into a convenient interface.
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREAMBLE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREAMBLE_H
 
+#include "CollectMacros.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "FS.h"
 #include "Headers.h"
 #include "index/CanonicalIncludes.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Tooling/CompilationDatabase.h"
 
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 
 /// The parsed preamble and associated data.
 ///
 /// As we must avoid re-parsing the preamble, any information that can only
 /// be obtained during parsing must be eagerly captured and stored here.
 struct PreambleData {
   PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags,
-               IncludeStructure Includes,
-               std::vector<std::string> MainFileMacros,
+               IncludeStructure Includes, MainFileMacros Macros,
                std::unique_ptr<PreambleFileStatusCache> StatCache,
                CanonicalIncludes CanonIncludes);
 
   tooling::CompileCommand CompileCommand;
   PrecompiledPreamble Preamble;
   std::vector<Diag> Diags;
   // Processes like code completions and go-to-definitions will need #include
   // information, and their compile action skips preamble range.
   IncludeStructure Includes;
   // Macros defined in the preamble section of the main file.
   // Users care about headers vs main-file, not preamble vs non-preamble.
   // These should be treated as main-file entities e.g. for code completion.
-  std::vector<std::string> MainFileMacros;
+  MainFileMacros Macros;
   // Cache of FS operations performed when building the preamble.
   // When reusing a preamble, this cache can be consumed to save IO.
   std::unique_ptr<PreambleFileStatusCache> StatCache;
   CanonicalIncludes CanonIncludes;
 };
 
 using PreambleParsedCallback =
     std::function<void(ASTContext &, std::shared_ptr<clang::Preprocessor>,

clang-tools-extra/trunk/clangd/Preamble.cpp

 
 bool compileCommandsAreEqual(const tooling::CompileCommand &LHS,
                              const tooling::CompileCommand &RHS) {
   // We don't check for Output, it should not matter to clangd.
   return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
          llvm::makeArrayRef(LHS.CommandLine).equals(RHS.CommandLine);
 }
 
-// This collects macro definitions in the *preamble region* of the main file.
-// (Contrast with CollectMainFileMacroExpansions in ParsedAST.cpp, which
-// collects macro *expansions* in the rest of the main file.
-class CollectMainFileMacros : public PPCallbacks {
-public:
-  explicit CollectMainFileMacros(const SourceManager &SM,
-                                 std::vector<std::string> *Out)
-      : SM(SM), Out(Out) {}
-
-  void FileChanged(SourceLocation Loc, FileChangeReason,
-                   SrcMgr::CharacteristicKind, FileID Prev) {
-    InMainFile = SM.isWrittenInMainFile(Loc);
-  }
-
-  void MacroDefined(const Token &MacroName, const MacroDirective *MD) {
-    if (InMainFile)
-      MainFileMacros.insert(MacroName.getIdentifierInfo()->getName());
-  }
-
-  void EndOfMainFile() {
-    for (const auto &Entry : MainFileMacros)
-      Out->push_back(Entry.getKey());
-    llvm::sort(*Out);
-  }
-
-private:
-  const SourceManager &SM;
-  bool InMainFile = true;
-  llvm::StringSet<> MainFileMacros;
-  std::vector<std::string> *Out;
-};
-
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
-      : File(File), ParsedCallback(ParsedCallback) {
-  }
+      : File(File), ParsedCallback(ParsedCallback) {}
 
   IncludeStructure takeIncludes() { return std::move(Includes); }
 
-  std::vector<std::string> takeMainFileMacros() {
-    return std::move(MainFileMacros);
-  }
+  MainFileMacros takeMacros() { return std::move(Macros); }
 
   CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); }
 
   void AfterExecute(CompilerInstance &CI) override {
     if (!ParsedCallback)
       return;
     trace::Span Tracer("Running PreambleCallback");
     ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr(), CanonIncludes);
   }
 
   void BeforeExecute(CompilerInstance &CI) override {
     CanonIncludes.addSystemHeadersMapping(CI.getLangOpts());
+    LangOpts = &CI.getLangOpts();
     SourceMgr = &CI.getSourceManager();
   }
 
   std::unique_ptr<PPCallbacks> createPPCallbacks() override {
-    assert(SourceMgr && "SourceMgr must be set at this point");
+    assert(SourceMgr && LangOpts &&
+           "SourceMgr and LangOpts must be set at this point");
+
     return std::make_unique<PPChainedCallbacks>(
         collectIncludeStructureCallback(*SourceMgr, &Includes),
-        std::make_unique<CollectMainFileMacros>(*SourceMgr, &MainFileMacros));
+        std::make_unique<CollectMainFileMacros>(*SourceMgr, *LangOpts, Macros));
   }
 
   CommentHandler *getCommentHandler() override {
     IWYUHandler = collectIWYUHeaderMaps(&CanonIncludes);
     return IWYUHandler.get();
   }
 
 private:
   PathRef File;
   PreambleParsedCallback ParsedCallback;
   IncludeStructure Includes;
   CanonicalIncludes CanonIncludes;
-  std::vector<std::string> MainFileMacros;
+  MainFileMacros Macros;
   std::unique_ptr<CommentHandler> IWYUHandler = nullptr;
-  SourceManager *SourceMgr = nullptr;
+  const clang::LangOptions *LangOpts = nullptr;
+  const SourceManager *SourceMgr = nullptr;
 };
 
 } // namespace
 
 PreambleData::PreambleData(PrecompiledPreamble Preamble,
                            std::vector<Diag> Diags, IncludeStructure Includes,
-                           std::vector<std::string> MainFileMacros,
+                           MainFileMacros Macros,
                            std::unique_ptr<PreambleFileStatusCache> StatCache,
                            CanonicalIncludes CanonIncludes)
     : Preamble(std::move(Preamble)), Diags(std::move(Diags)),
-      Includes(std::move(Includes)), MainFileMacros(std::move(MainFileMacros)),
+      Includes(std::move(Includes)), Macros(std::move(Macros)),
       StatCache(std::move(StatCache)), CanonIncludes(std::move(CanonIncludes)) {
 }
 
 std::shared_ptr<const PreambleData>
 buildPreamble(PathRef FileName, CompilerInvocation &CI,
               std::shared_ptr<const PreambleData> OldPreamble,
               const tooling::CompileCommand &OldCompileCommand,
               const ParseInputs &Inputs, bool StoreInMemory,
buildPreamble(PathRef FileName, CompilerInvocation &CI,
 
   if (BuiltPreamble) {
     vlog("Built preamble of size {0} for file {1}", BuiltPreamble->getSize(),
          FileName);
     std::vector<Diag> Diags = PreambleDiagnostics.take();
     return std::make_shared<PreambleData>(
         std::move(*BuiltPreamble), std::move(Diags),
         SerializedDeclsCollector.takeIncludes(),
-        SerializedDeclsCollector.takeMainFileMacros(), std::move(StatCache),
+        SerializedDeclsCollector.takeMacros(), std::move(StatCache),
         SerializedDeclsCollector.takeCanonicalIncludes());
   } else {
     elog("Could not build a preamble for file {0}", FileName);
     return nullptr;
   }
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp

 public:
   HighlightingTokenCollector(ParsedAST &AST) : AST(AST) {}
 
   std::vector<HighlightingToken> collectTokens() {
     Tokens.clear();
     TraverseAST(AST.getASTContext());
     // Add highlightings for macro expansions as they are not traversed by the
     // visitor.
-    for (SourceLocation Loc : AST.getMacros())
-      addToken(Loc, HighlightingKind::Macro);
+    for (const auto &M : AST.getMacros().Ranges)
+      Tokens.push_back({HighlightingKind::Macro, M});
+    // addToken(Loc, HighlightingKind::Macro);
     // Initializer lists can give duplicates of tokens, therefore all tokens
     // must be deduplicated.
     llvm::sort(Tokens);
     auto Last = std::unique(Tokens.begin(), Tokens.end());
     Tokens.erase(Last, Tokens.end());
     // Macros can give tokens that have the same source range but conflicting
     // kinds. In this case all tokens sharing this source range should be
     // removed.

clang-tools-extra/trunk/clangd/unittests/ParsedASTTests.cpp

TEST(ParsedASTTest, CanBuildInvocationWithUnknownArgs) {
   // Unknown forwarded to -cc1 should not a failure either.
   Inputs.CompileCommand.CommandLine = {
       "clang", "-Xclang", "-fsome-unknown-flag", testPath("foo.cpp")};
   EXPECT_NE(buildCompilerInvocation(Inputs, IgnoreDiags), nullptr);
 }
 
 TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
   Annotations TestCase(R"cpp(
-    #define MACRO_ARGS(X, Y) X Y
-    // - premable ends, macros inside preamble are not considered in main file.
+    #define ^MACRO_ARGS(X, Y) X Y
+    // - preamble ends
     ^ID(int A);
     // Macro arguments included.
     ^MACRO_ARGS(^MACRO_ARGS(^MACRO_EXP(int), A), ^ID(= 2));
 
     // Macro names inside other macros not included.
     #define ^MACRO_ARGS2(X, Y) X Y
     #define ^FOO BAR
     #define ^BAR 1
TU.HeaderCode = R"cpp(
     MACRO_EXP(int B);
   )cpp";
   TU.AdditionalFiles["foo.inc"] = R"cpp(
     int C = ID(1);
     #define DEF 1
     int D = DEF;
   )cpp";
   ParsedAST AST = TU.build();
-  const std::vector<SourceLocation> &MacroExpansionLocations = AST.getMacros();
   std::vector<Position> MacroExpansionPositions;
-  for (const auto &L : MacroExpansionLocations)
-    MacroExpansionPositions.push_back(
-        sourceLocToPosition(AST.getSourceManager(), L));
-  EXPECT_EQ(MacroExpansionPositions, TestCase.points());
+  for (const auto &R : AST.getMacros().Ranges)
+    MacroExpansionPositions.push_back(R.start);
+  EXPECT_THAT(MacroExpansionPositions,
+              testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

)cpp",
         $Class[[F]] $LocalVariable[[FF]];
         $Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $LocalVariable[[GG]];
         $LocalVariable[[GG]].$Method[[foo]](&$LocalVariable[[FF]]);
         $Class[[A]]<$Function[[foo]]> $LocalVariable[[AA]];
     )cpp",
       // Tokens that share a source range but have conflicting Kinds are not
       // highlighted.
       R"cpp(
-      #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
-      #define DEF_CLASS(T) class T {};
+      #define $Macro[[DEF_MULTIPLE]](X) namespace X { class X { int X; }; }
+      #define $Macro[[DEF_CLASS]](T) class T {};
       // Preamble ends.
       $Macro[[DEF_MULTIPLE]](XYZ);
       $Macro[[DEF_MULTIPLE]](XYZW);
       $Macro[[DEF_CLASS]]($Class[[A]])
       #define $Macro[[MACRO_CONCAT]](X, V, T) T foo##X = V
       #define $Macro[[DEF_VAR]](X, V) int X = V
       #define $Macro[[DEF_VAR_T]](T, X, V) T X = V
       #define $Macro[[DEF_VAR_REV]](V, X) DEF_VAR(X, V)
)cpp",
 
       #define $Macro[[CALL_FN]](F) F();
       #define $Macro[[DEF_FN]](F) void F ()
       $Macro[[DEF_FN]]($Function[[g]]) {
         $Macro[[CALL_FN]]($Function[[foo]]);
       }
     )cpp",
       R"cpp(
-      #define fail(expr) expr
-      #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+      #define $Macro[[fail]](expr) expr
+      #define $Macro[[assert]](COND) if (!(COND)) { fail("assertion failed" #COND); }
       // Preamble ends.
       $Primitive[[int]] $Variable[[x]];
       $Primitive[[int]] $Variable[[y]];
       $Primitive[[int]] $Function[[f]]();
       $Primitive[[void]] $Function[[foo]]() {
         $Macro[[assert]]($Variable[[x]] != $Variable[[y]]);
         $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
       }
EXPECT_THAT(ActualDiff,
               testing::UnorderedElementsAre(
                   testing::AllOf(LineNumber(3), EmptyHighlightings()),
                   testing::AllOf(LineNumber(4), EmptyHighlightings())));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang
 No newline at end of file