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

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D60463

Files:
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -29,6 +29,8 @@
 #include "gmock/gmock.h"
 #include "llvm/ADT/StringMap.h"
 
+#include <regex>
+
 namespace clang {
 namespace ast_matchers {
 
@@ -56,6 +58,81 @@
                                    llvm::MemoryBuffer::getMemBuffer(Code));
 }
 
+void checkImportedSourceLocations(const Decl *FromD, const Decl *ToD) {
+  // FIXME: Remove this line when all failures are fixed.
+  return;
+
+  // Check for matching source locations in From and To AST.
+  // FIXME: The check can be improved by using AST visitor and manually check
+  // all source locations for equality.
+  // (That check can be made more general by checking for other attributes.)
+
+  // 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);
+  // The AST dump additionally traverses the AST and can catch certain bugs like
+  // poorly or not implemented subtrees.
+
+  // 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());
+  auto ToLoc =
+      std::sregex_iterator(ToString.begin(), ToString.end(), MatchSourceLoc);
+  auto FromLoc = std::sregex_iterator(FromString.begin(), FromString.end(),
+                                      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 the from AST is bigger it may have a matching prefix, ignore this case:
+  // ToLoc == std::sregex_iterator() && FromLoc != std::sregex_iterator()
+
+  // If the To AST is bigger (or has more source locations), indicate error.
+  if (FromLoc == std::sregex_iterator() && ToLoc != std::sregex_iterator())
+    SourceLocationMismatch = true;
+
+  EXPECT_FALSE(SourceLocationMismatch);
+}
+
 const StringRef DeclToImportID = "declToImport";
 const StringRef DeclToVerifyID = "declToVerify";
 
@@ -112,6 +189,8 @@
       // This traverses the AST to catch certain bugs like poorly or not
       // implemented subtrees.
       (*Imported)->dump(ToNothing);
+      // More detailed source location checks.
+      checkImportedSourceLocations(Node, *Imported);
     }
 
     return Imported;
@@ -480,7 +559,10 @@
     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;
   }
 
   template <class DeclT> DeclT *Import(DeclT *From, Language Lang) {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits