Don't pass uninitialized QueryKind

[vitalybuka]

Summary

Even if findImplementors does not use
uninitialized parameter it's still UB and
it's going to be detected by msan with:
-Xclang -enable-noundef-analysis -mllvm -msan-eager-checks=1

[kda]

This seems to introduce a new branch, should there be a new unit test in: XRefsTests.cpp?

clang-tools-extra/clangd/XRefs.cpp

1313

It seems like the first line of 'findImplementors' is 'if (IDs.empty() || !Index)`.
I wonder if the correct fix is to drop the '!Index' check in findImplementors.

[kda]

I think I forgot to 'Request Changes'.

[vitalybuka]

In D116827#3228524, @kda wrote:

This seems to introduce a new branch, should there be a new unit test in: XRefsTests.cpp?

It's not functional change, test should not be able to see a difference after this patch.

clang-tools-extra/clangd/XRefs.cpp

1313

Sorry, I am no following how "Index" is related to uninitialized QueryKind?

[vitalybuka]

In D116827#3228594, @vitalybuka wrote:

In D116827#3228524, @kda wrote:

This seems to introduce a new branch, should there be a new unit test in: XRefsTests.cpp?

It's not functional change, test should not be able to see a difference after this patch.

to clarify: it's not realy new branch, as findImplementors already does exactly the same check.
I considered removing that check, but it will complicated other callers.

[vitalybuka]

clang-tools-extra/clangd/XRefs.cpp

311

here

[kda]

clang-tools-extra/clangd/XRefs.cpp

1313

Oh. Ignore some of my previous discussion, I missed the summary.

Why not just initialize QueryKind?
That seems like a more direct solution for the specific issue.
I believe it would be safe, as IDs is only inserted into when QueryKind is explicitly set.

[vitalybuka]

update

clang-tools-extra/clangd/XRefs.cpp

const NamedDecl *getPreferredDecl(const NamedDecl *D) {
 
   return D;
 }
 
 std::vector<LocatedSymbol> findImplementors(llvm::DenseSet<SymbolID> IDs,
                                             RelationKind Predicate,
                                             const SymbolIndex *Index,
                                             llvm::StringRef MainFilePath) {
   if (IDs.empty() || !Index)
     return {};
   static constexpr trace::Metric FindImplementorsMetric(
       "find_implementors", trace::Metric::Counter, "case");
   switch (Predicate) {
   case RelationKind::BaseOf:
     FindImplementorsMetric.record(1, "find-base");
     break;
   case RelationKind::OverriddenBy:
std::vector<LocatedSymbol> findImplementations(ParsedAST &AST, Position Pos,
   if (!CurLoc) {
     elog("Failed to convert position to source location: {0}",
          CurLoc.takeError());
     return {};
   }
   DeclRelationSet Relations =
       DeclRelation::TemplatePattern | DeclRelation::Alias;
   llvm::DenseSet<SymbolID> IDs;
-  RelationKind QueryKind;
+  RelationKind QueryKind = RelationKind::OverriddenBy;
   for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations)) {
     if (const auto *CXXMD = llvm::dyn_cast<CXXMethodDecl>(ND)) {
       if (CXXMD->isVirtual()) {
         IDs.insert(getSymbolID(ND));
         QueryKind = RelationKind::OverriddenBy;
       }
     } else if (const auto *RD = dyn_cast<CXXRecordDecl>(ND)) {
       IDs.insert(getSymbolID(RD));
       QueryKind = RelationKind::BaseOf;
     }
   }
   return findImplementors(std::move(IDs), QueryKind, Index, *MainFilePath);
 }
 
 namespace {
 // Recursively finds all the overridden methods of `CMD` in complete type
 // hierarchy.
 void getOverriddenMethods(const CXXMethodDecl *CMD,
                           llvm::DenseSet<SymbolID> &OverriddenMethods) {
   if (!CMD)