[PATCH] D49012: [clangd] Uprank delcarations when "using q::name" is present in the main file

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/Quality.cpp:48
+  if (R.ShadowDecl) {
+    const auto Loc = SourceMgr.getSpellingLoc(R.ShadowDecl->getLocation());
+    if (SourceMgr.isWrittenInMainFile(Loc))
----------------
ioeric wrote:
> I think we might want to use `getExpansionLoc` here. Consider `DEFINE_string` which is implemented like:
> ```
> #define DEFINE_string(XXX) \
> namespace FLs { \
> SomeType FLAGS_XXX; \
> } \
> using FLs::FLAGS_XXX;
> ```
> When you have `DEFINE_string(X) in the main file, the spelling location will be in the file where the macro is defined, while the expansion location will be in the main file, and I think we would want later.
> 
If you're not motivated to add the test now, I doubt anyone ever will be, so just remove the FIXME.


================
Comment at: clang-tools-extra/unittests/clangd/QualityTests.cpp:116
   EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0f)
-      << "Current file and header";
+      << "Current file and definition in header";
+
----------------
(this still says "definition", which doesn't seem true/relevant)


================
Comment at: clang-tools-extra/unittests/clangd/QualityTests.cpp:119
+  Relevance = {};
+  const auto SymbolName = "Bar";
+  const auto *Shadow =
----------------
nit: inline this?
(or if you really want to name it, give a more specific name)


https://reviews.llvm.org/D49012