[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.
balazske updated this revision to Diff 202414.
balazske added a comment.
- Using size_t instead of int.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60463/new/
https://reviews.llvm.org/D60463
Files:
unittests/AST/ASTImporterFixtures.cpp
unittests/AST/ASTImporterFixtures.h
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -49,9 +49,8 @@
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;
Index: unittests/AST/ASTImporterFixtures.h
===
--- unittests/AST/ASTImporterFixtures.h
+++ unittests/AST/ASTImporterFixtures.h
@@ -42,6 +42,8 @@
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.
Index: unittests/AST/ASTImporterFixtures.cpp
===
--- unittests/AST/ASTImporterFixtures.cpp
+++ unittests/AST/ASTImporterFixtures.cpp
@@ -18,6 +18,8 @@
#include "clang/Frontend/ASTUnit.h"
#include "clang/Tooling/Tooling.h"
+#include
+
namespace clang {
namespace ast_matchers {
@@ -38,6 +40,82 @@
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:
+ // ::
+ // or
+ // line::
+ // or
+ // col:
+ // or
+ // ''
+ // 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(
+ "|((\\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(FileNa
[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.
balazske added a comment. In the current state there are failing AST tests. This test can be added after the problems are fixed. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60463/new/ https://reviews.llvm.org/D60463 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.
balazske updated this revision to Diff 202197.
balazske added a comment.
New patch and check the first line of AST dump only.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60463/new/
https://reviews.llvm.org/D60463
Files:
unittests/AST/ASTImporterFixtures.cpp
unittests/AST/ASTImporterFixtures.h
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -49,9 +49,8 @@
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;
Index: unittests/AST/ASTImporterFixtures.h
===
--- unittests/AST/ASTImporterFixtures.h
+++ unittests/AST/ASTImporterFixtures.h
@@ -42,6 +42,8 @@
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.
Index: unittests/AST/ASTImporterFixtures.cpp
===
--- unittests/AST/ASTImporterFixtures.cpp
+++ unittests/AST/ASTImporterFixtures.cpp
@@ -18,6 +18,8 @@
#include "clang/Frontend/ASTUnit.h"
#include "clang/Tooling/Tooling.h"
+#include
+
namespace clang {
namespace ast_matchers {
@@ -38,6 +40,82 @@
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:
+ // ::
+ // or
+ // line::
+ // or
+ // col:
+ // or
+ // ''
+ // 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(
+ "|((\\w|\\.)+):\\d+:\\d+|line:\\d+:\\d+|col:\\d+");
+
+ std::string ToString(ToStream.str());
+ std::string FromString(FromStream.str());
+ int ToEndl = ToString.find('\n');
+ if (ToEndl == std::string::npos)
+ToEndl = ToString.size();
+ int 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),
[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.
balazske added a comment. 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. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60463/new/ https://reviews.llvm.org/D60463 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.
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
+
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:
+ // ::
+ // or
+ // line::
+ // or
+ // col:
+ // or
+ // ''
+ // 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(
+ "|((\\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 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
