[ASTImporter] Import overrides before importing the rest of the chain

[martong]

Summary

During method import we check for structural eq of two methods.
In the structural eq check we check for their isVirtual() flag. That
flag, however, may depend on the number of overrides. Before this
change we imported the overrides *after* we had imported the rest of the
redecl chain. So, during the import of another decl from the chain
IsVirtual() gave false result.

Writing tests for this is not really possible, because there is no way
to remove an overridden method via the AST API.
(We should access the private ASTContext::OverriddenMethods container.)
Also, we should do the remove in the middle of the import process.

[a_sidorin]

Hello Gabor,

The change looks harmless so I think it can be accepted even without tests. Did you encountered the issue while analyzing some real code?

[martong]

Hi Aleksei,

Thanks for the review. Yes, we encountered the issue during the analysis of Xerces.

lib/AST/ASTImporter.cpp

ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
   // We want the friend functions to be found in the semantic context by lookup.
   // FIXME should we handle this generically in VisitFriendDecl?
   // In Other cases when LexicalDC != DC we don't want it to be added,
   // e.g out-of-class definitions like void B::f() {} .
   if (LexicalDC != DC && IsFriend) {
     DC->makeDeclVisibleInContext(ToFunction);
   }
 
+  if (auto *FromCXXMethod = dyn_cast<CXXMethodDecl>(D))
+    ImportOverrides(cast<CXXMethodDecl>(ToFunction), FromCXXMethod);
+
   // Import the rest of the chain. I.e. import all subsequent declarations.
   for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt) {
     ExpectedDecl ToRedeclOrErr = import(*RedeclIt);
     if (!ToRedeclOrErr)
       return ToRedeclOrErr.takeError();
   }
 
-  if (auto *FromCXXMethod = dyn_cast<CXXMethodDecl>(D))
-    ImportOverrides(cast<CXXMethodDecl>(ToFunction), FromCXXMethod);
-
   return ToFunction;
 }
 
 ExpectedDecl ASTNodeImporter::VisitCXXMethodDecl(CXXMethodDecl *D) {
   return VisitFunctionDecl(D);
 }
 
 ExpectedDecl ASTNodeImporter::VisitCXXConstructorDecl(CXXConstructorDecl *D) {