[clang][ASTImporter] Remove use of ParentMapContext.

[balazske]

Summary

Function 'isAncestorDeclContextOf' was using 'ParentMapContext' for
looking up parent of statement nodes. There may be cases (bugs?) with
ParentMapContext when parents of specific statements are not found.
This leads to 'ASTImporter' infinite import loops when function
'hasAutoReturnTypeDeclaredInside' returns false incorrectly.
A real case was found but could not be reproduced with test code.
Use of 'ParentMapContext' is now removed and changed to a more safe
(currently) method by searching for declarations in statements
and find parent of these declarations. The new code was tested on
a number of projects and no related crash was found.

[martong]

It is very good that we can get rid of the ParentMapContext because it was sub-optimal to build that up for all declaration contexts of the translation unit. Now we discover the parent-child relations for ourselves and only for the needed declaration contexts. This is the proper way to go.

[balazske]

This is really a NFC-like change but not NFC because it has visible effects of removing some crashes. I could not produce a test that provokes the wrong case (when getParents returns an empty list). Is it enough to have no new test here? If we can get a test for the crash it needs more investigation to check if RecursiveASTVisitor works correct and probably a fix at other place.

[martong]

In D136684#3884983, @balazske wrote:

This is really a NFC-like change but not NFC because it has visible effects of removing some crashes. I could not produce a test that provokes the wrong case (when getParents returns an empty list). Is it enough to have no new test here? If we can get a test for the crash it needs more investigation to check if RecursiveASTVisitor works correct and probably a fix at other place.

In my opinion, this case a very rare case when the change is justified even without the tests. Of course, it would be better if you had a test that is a minimal reproducer for the crash you are talking about. But, since you remove a sub-optimal infrastructural element, that is the parent map, which relies on the mentioned RecursiveASTVisitor, I don't think that we need to hunt down bugs for those components that you just dumped out from the ASTImporter code.

clang/lib/AST/ASTImporter.cpp

 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OperationKinds.h"
-#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
static bool isAncestorDeclContextOf(const DeclContext *DC, const Decl *D) {
   while (DCi != D->getTranslationUnitDecl()) {
     if (DCi == DC)
       return true;
     DCi = DCi->getParent();
   }
   return false;
 }
 
-// Returns true if the statement S has a parent declaration that has a
-// DeclContext that is inside (or equal to) DC. In a specific use case if DC is
-// a FunctionDecl, check if statement S resides in the body of the function.
+// Check if there is a declaration that has 'DC' as parent context and is
+// referenced from statement 'S' or one of its children. The search is done in
+// BFS order through children of 'S'.
 static bool isAncestorDeclContextOf(const DeclContext *DC, const Stmt *S) {
-  ParentMapContext &ParentC = DC->getParentASTContext().getParentMapContext();
-  DynTypedNodeList Parents = ParentC.getParents(*S);
-  while (!Parents.empty()) {
-    if (const Decl *PD = Parents.begin()->get<Decl>())
-      return isAncestorDeclContextOf(DC, PD);
-    Parents = ParentC.getParents(*Parents.begin());
+  SmallVector<const Stmt *> ToProcess;
+  ToProcess.push_back(S);
+  while (!ToProcess.empty()) {
+    const Stmt *CurrentS = ToProcess.pop_back_val();
+    ToProcess.append(CurrentS->child_begin(), CurrentS->child_end());
+    if (const auto *DeclRef = dyn_cast<DeclRefExpr>(CurrentS))
+      if (const Decl *D = DeclRef->getDecl())
+        if (isAncestorDeclContextOf(DC, D))
+          return true;
   }
   return false;
 }
 
 namespace {
 /// Check if a type has any reference to a declaration that is inside the body
 /// of a function.
 /// The \c CheckType(QualType) function should be used to determine