[ASTImporter] Properly delete decls from SavedImportPaths

[jarin]

Summary

We see a significant regression (~40% slower on large codebases) in expression evaluation after https://reviews.llvm.org/rL364771. A sampling profile shows the extra time is spent in SavedImportPathsTy::operator[] when called from ASTImporter::Import. I believe this is because ASTImporter::Import adds an element to the SavedImportPaths map for each decl unconditionally (see https://github.com/llvm/llvm-project/blob/7b81c3f8793d30a4285095a9b67dcfca2117916c/clang/lib/AST/ASTImporter.cpp#L8256).

To fix this, we call SavedImportPathsTy::erase on the declaration rather than clearing its value vector. That way we do not accidentally introduce new empty elements. (With this patch the performance is restored, and we do not see SavedImportPathsTy::operator[] in the profile anymore.)

[martong]

Thanks for the fix! Looks good to me!

[jarin]

Thanks for the review!

I am not a committer; could you land the fix for me?

[teemperor]

I can land it, seems to pass all LLDB tests too.

clang/lib/AST/ASTImporter.cpp

for (const auto &Path : SavedImportPaths[FromD])
         //FIXME Should we remove these Decls from ImportedDecls?
         // Set the error for the mapped to Decl, which is in the "to" context.
         auto Ii = ImportedDecls.find(FromDi);
         if (Ii != ImportedDecls.end())
           SharedState->setImportDeclError(Ii->second, ErrOut);
           // FIXME Should we remove these Decls from the LookupTable,
           // and from ImportedFromDecls?
       }
-    SavedImportPaths[FromD].clear();
+    SavedImportPaths.erase(FromD);
 
     // Do not return ToDOrErr, error was taken out of it.
     return make_error<ImportError>(ErrOut);
   }
 
   ToD = *ToDOrErr;
 
   // FIXME: Handle the "already imported with error" case. We can get here
Expected<Decl *> ASTImporter::Import(Decl *FromD) {
 
   // Make sure that ImportImpl registered the imported decl.
   assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
 
   // Notify subclasses.
   Imported(FromD, ToD);
 
   updateFlags(FromD, ToD);
-  SavedImportPaths[FromD].clear();
+  SavedImportPaths.erase(FromD);
   return ToDOrErr;
 }
 
 Expected<DeclContext *> ASTImporter::ImportContext(DeclContext *FromDC) {
   if (!FromDC)
     return FromDC;
 
   ExpectedDecl ToDCOrErr = Import(cast<Decl>(FromDC));