[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 marked an inline comment as done.
usaxena95 added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:28
+      )cpp",
+      // FIXME: Locating macro in duplicate definitions doesn't work. Enable
+      // this once LocateMacro is fixed.
----------------
hokein wrote:
> This is interesting, by "doesn't work", you mean the function could not locate the correct definitions for (the first & second `abc`)?
It is not able to locate the macro for second definition.  
Causes assertion failure at line 93:  assert(Macro);


================
Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:74
+    for (int I = 1;; I++) {
+      const auto ExpectedRefs = T.ranges(llvm::to_string(I));
+      if (ExpectedRefs.empty())
----------------
hokein wrote:
> nit: const auto &
ranges returns a copy and not a reference. I would expect copy elision here and  & would be unnecessary. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70008/new/

https://reviews.llvm.org/D70008