[clang] Fix a crash during code completion

[adamcz]

Summary

During code completion, lookupInDeclContext() calls
CodeCompletionDeclConsumer::FoundDecl(),which can mutate StoredDeclsMap,
over which lookupInDeclContext() iterates. This can lead to invalidation
of iterators and an assert()-crash.

Example code where this happens:
#include <list>
int main() {

std::list<int>;
std::^

}
with code completion on ^ with -std=c++20.

I do not have a repro case that does not need standard library.

This fix stores pointers to NamedDecls in a temporary vector, then
visits them outside of the main loop, when StoredDeclsMap iterators are
gone.

[adamcz]

This is https://github.com/clangd/clangd/issues/771

Sending for review mostly to see if you have any comments on this approach. Trying to reproduce this without <list>, <vector> or something like this didn't work for me so far, I must be holding it wrong or something ;-) Still, submitting without a nice test case seems wrong.
The whole C++20, as far as I can tell, simply adds a bunch of code that's behind ifdef, which is what causes the crash. I don't think this is related to any C++20 features.

[kadircet]

we discussed offline but i forgot to stamp this one. it would be nice to have a test case, but fix is relatively safe and getting a repro turned out to be hard (since it depends on a densemap growing).

thanks!

clang/lib/Sema/SemaLookup.cpp

if (isa<TranslationUnitDecl>(Ctx) &&
       }
 
       return;
     }
 
     if (CXXRecordDecl *Class = dyn_cast<CXXRecordDecl>(Ctx))
       Result.getSema().ForceDeclarationOfImplicitMembers(Class);
 
+    llvm::SmallVector<NamedDecl *, 4> DeclsToVisit;
     // We sometimes skip loading namespace-level results (they tend to be huge).
     bool Load = LoadExternal ||
                 !(isa<TranslationUnitDecl>(Ctx) || isa<NamespaceDecl>(Ctx));
     // Enumerate all of the results in this context.
     for (DeclContextLookupResult R :
          Load ? Ctx->lookups()
               : Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
       for (auto *D : R) {
         if (auto *ND = Result.getAcceptableDecl(D)) {
-          Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
-          Visited.add(ND);
-        }
-      }
+          // Rather than visit immediatelly, we put ND into a vector and visit
+          // all decls, in order, outside of this loop. The reason is that
+          // Consumer.FoundDecl() may invalidate the iterators used in the two
+          // loops above.
+          DeclsToVisit.push_back(ND);
+        }
+
+    for (auto *ND : DeclsToVisit) {
+      Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
+      Visited.add(ND);
     }
+    DeclsToVisit.clear();
 
     // Traverse using directives for qualified name lookup.
     if (QualifiedNameLookup) {
       ShadowContextRAII Shadow(Visited);
       for (auto I : Ctx->using_directives()) {
         if (!Result.getSema().isVisible(I))
           continue;
         lookupInDeclContext(I->getNominatedNamespace(), Result,