[ASTImporter] Add check for correct import of source locations.

[balazske]

Summary

Unit tests extended with a new check for source locations.
Currently a simple match check in the node text dumps is done.
The new check is disabled for now, until the failures are fixed.

[balazske]

This test will work theoretically only if the order of every imported Decl (and not only FieldDecl) is correct, this is not the case now. So probably a better solution for the problem should be found: Enumerate and match (the From and To) SourceLocations with AST visitor. There should be some existing code that is doing somewhat similar in clang-query but I did not find it.

[balazske]

New patch and check the first line of AST dump only.

[balazske]

In the current state there are failing AST tests. This test can be added after the problems are fixed.

[balazske]

• Using size_t instead of int.

unittests/AST/ASTImporterFixtures.h

 // Creates a virtual file and assigns that to the context of given AST. If the
 // file already exists then the file will not be created again as a duplicate.
 void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
                                std::unique_ptr<llvm::MemoryBuffer> &&Buffer);
 
 void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
                                StringRef Code);
 
+void checkImportedSourceLocations(const Decl *FromD, const Decl *ToD);
+
 // Common base for the different families of ASTImporter tests that are
 // parameterized on the compiler options which may result a different AST. E.g.
 // -fms-compatibility or -fdelayed-template-parsing.
 class CompilerOptionSpecificTest : public ::testing::Test {
 protected:
   // Return the extra arguments appended to runtime options at compilation.
   virtual ArgVector getExtraArgs() const { return ArgVector(); }
 

unittests/AST/ASTImporterFixtures.cpp

 
 #include "ASTImporterFixtures.h"
 
 #include "clang/AST/ASTImporter.h"
 #include "clang/AST/ASTImporterLookupTable.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Tooling/Tooling.h"
 
+#include <regex>
+
 namespace clang {
 namespace ast_matchers {
 
 void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
                                std::unique_ptr<llvm::MemoryBuffer> &&Buffer) {
   assert(ToAST);
   ASTContext &ToCtx = ToAST->getASTContext();
   auto *OFS = static_cast<llvm::vfs::OverlayFileSystem *>(
       &ToCtx.getSourceManager().getFileManager().getVirtualFileSystem());
   auto *MFS = static_cast<llvm::vfs::InMemoryFileSystem *>(
       OFS->overlays_begin()->get());
   MFS->addFile(FileName, 0, std::move(Buffer));
 }
 
 void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
                                StringRef Code) {
   return createVirtualFileIfNeeded(ToAST, FileName,
                                    llvm::MemoryBuffer::getMemBuffer(Code));
 }
 
+void checkImportedSourceLocations(const Decl *FromD, const Decl *ToD) {
+  // Check AST dump for matching source locations in From and To AST.
+  // The first line of the AST dump contains information about the root node and
+  // some of the SourceLocation info, but certainly not everything.
+  // Still it is worth to check these. The test can be extended to check the
+  // whole AST dump but for this to work the ordering of imported Decls must be
+  // the same which is not the case now.
+  // The AST dump additionally traverses the AST and can catch certain bugs like
+  // poorly or not implemented subtrees.
+
+  // Print debug information?
+  const bool Print = false;
+
+  SmallString<1024> ToPrinted;
+  SmallString<1024> FromPrinted;
+  llvm::raw_svector_ostream ToStream(ToPrinted);
+  llvm::raw_svector_ostream FromStream(FromPrinted);
+
+  ToD->dump(ToStream);
+  FromD->dump(FromStream);
+
+  // search for SourceLocation strings:
+  // <filename>:<line>:<col>
+  // or
+  // line:<line>:<col>
+  // or
+  // col:<col>
+  // or
+  // '<invalid sloc>'
+  // If a component (filename or line) is same as in the last location
+  // it is not printed.
+  // Filename component is grouped into sub-expression to make it extractable.
+  std::regex MatchSourceLoc(
+      "<invalid sloc>|((\\w|\\.)+):\\d+:\\d+|line:\\d+:\\d+|col:\\d+");
+
+  std::string ToString(ToStream.str());
+  std::string FromString(FromStream.str());
+  size_t ToEndl = ToString.find('\n');
+  if (ToEndl == std::string::npos)
+    ToEndl = ToString.size();
+  size_t FromEndl = FromString.find('\n');
+  if (FromEndl == std::string::npos)
+    FromEndl = FromString.size();
+  auto ToLoc = std::sregex_iterator(ToString.begin(), ToString.begin() + ToEndl,
+                                    MatchSourceLoc);
+  auto FromLoc = std::sregex_iterator(
+      FromString.begin(), FromString.begin() + FromEndl, MatchSourceLoc);
+  if (Print) {
+    llvm::errs() << ToString << "\n\n\n" << FromString << "\n";
+    llvm::errs() << "----\n";
+  }
+  if (ToLoc->size() > 1 && FromLoc->size() > 1 && (*ToLoc)[1] != (*FromLoc)[1])
+    // Different filenames in To and From.
+    // This should mean that a to-be-imported decl was mapped to an existing
+    // (these normally reside in different files) and the check is
+    // not applicable.
+    return;
+
+  bool SourceLocationMismatch = false;
+  while (ToLoc != std::sregex_iterator() && FromLoc != std::sregex_iterator()) {
+    if (Print)
+      llvm::errs() << ToLoc->str() << "|" << FromLoc->str() << "\n";
+    SourceLocationMismatch =
+        SourceLocationMismatch || (ToLoc->str() != FromLoc->str());
+    ++ToLoc;
+    ++FromLoc;
+  }
+  if (Print)
+    llvm::errs() << "----\n";
+
+  if (FromLoc != std::sregex_iterator() || ToLoc != std::sregex_iterator())
+    SourceLocationMismatch = true;
+
+  EXPECT_FALSE(SourceLocationMismatch);
+}
+
 ASTImporterTestBase::TU::TU(StringRef Code, StringRef FileName, ArgVector Args,
                             ImporterConstructor C)
     : Code(Code), FileName(FileName),
       Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args, this->FileName)),
       TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C) {
   Unit->enableSourceFileDiagnostics();
 
   // If the test doesn't need a specific ASTImporter, we just create a
TranslationUnitDecl *ASTImporterTestBase::getToTuDecl(StringRef ToSrcCode,
   lazyInitToAST(ToLang, ToSrcCode, OutputFileName);
   return ToAST->getASTContext().getTranslationUnitDecl();
 }
 
 Decl *ASTImporterTestBase::Import(Decl *From, Language ToLang) {
   lazyInitToAST(ToLang, "", OutputFileName);
   TU *FromTU = findFromTU(From);
   assert(LookupTablePtr);
-  return FromTU->import(*LookupTablePtr, ToAST.get(), From);
+  Decl *To = FromTU->import(*LookupTablePtr, ToAST.get(), From);
+  if (To)
+    checkImportedSourceLocations(From, To);
+  return To;
 }
 
 QualType ASTImporterTestBase::ImportType(QualType FromType, Decl *TUDecl,
                                          Language ToLang) {
   lazyInitToAST(ToLang, "", OutputFileName);
   TU *FromTU = findFromTU(TUDecl);
   assert(LookupTablePtr);
   return FromTU->import(*LookupTablePtr, ToAST.get(), FromType);

unittests/AST/ASTImporterTest.cpp

llvm::Expected<NodeType> importNode(ASTUnit *From, ASTUnit *To,
 
     if (Imported) {
       // This should dump source locations and assert if some source locations
       // were not imported.
       SmallString<1024> ImportChecker;
       llvm::raw_svector_ostream ToNothing(ImportChecker);
       ToCtx.getTranslationUnitDecl()->print(ToNothing);
 
-      // This traverses the AST to catch certain bugs like poorly or not
-      // implemented subtrees.
-      (*Imported)->dump(ToNothing);
+      // Additional SourceLocation checks.
+      checkImportedSourceLocations(Node, *Imported);
     }
 
     return Imported;
   }
 
   template <typename NodeType>
   testing::AssertionResult
   testImport(const std::string &FromCode, const ArgVector &FromArgs,